Apply row-level locking when updating stock item quantity (#12385)

src/backend/InvenTree/stock/models.py

@@ -1360,6 +1360,7 @@ class StockItem(
         # Delete outstanding BuildOrder allocations
         self.allocations.all().delete()
 
+    @transaction.atomic
     def allocateToCustomer(
         self, customer, quantity=None, order=None, user=None, notes=None
     ):
@@ -1376,6 +1377,10 @@ class StockItem(
             user: User that performed the action
             notes: Notes field
         """
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            raise ValidationError(_('Stock item no longer exists'))
+
         if quantity is None or self.serialized:
             quantity = self.quantity
 
@@ -1715,6 +1720,10 @@ class StockItem(
             notes: Any notes associated with the operation
             build: The BuildOrder to associate with the operation (optional)
         """
+        # Lock the database row, so concurrent adjustments are serialized
+        if not other_item.lock_quantity():
+            raise ValidationError(_('Stock item no longer exists'))
+
         try:
             quantity = Decimal(quantity)
         except (InvalidOperation, TypeError):
@@ -1934,6 +1943,10 @@ class StockItem(
         if quantity <= 0:
             raise ValidationError({'quantity': _('Quantity must be greater than zero')})
 
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            raise ValidationError(_('Stock item no longer exists'))
+
         if quantity > self.quantity:
             raise ValidationError({
                 'quantity': _('Quantity must not exceed available stock quantity')
@@ -2253,6 +2266,10 @@ class StockItem(
         if quantity <= 0:
             raise ValidationError({'quantity': _('Quantity must be greater than zero')})
 
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            raise ValidationError(_('Stock item no longer exists'))
+
         if quantity > self.quantity:
             raise ValidationError({
                 'quantity': _(
@@ -2514,6 +2531,29 @@ class StockItem(
         if len(other_items) == 0:
             return
 
+        # Lock all database rows (in pk order, to avoid deadlocks between
+        # concurrent merges) and refresh the quantity of each item
+        items_to_merge = sorted([self, *other_items], key=lambda item: item.pk)
+
+        locked_quantities = dict(
+            StockItem.objects
+            .select_for_update()
+            .filter(pk__in=[item.pk for item in items_to_merge])
+            .values_list('pk', 'quantity')
+        )
+
+        for item in items_to_merge:
+            if item.pk not in locked_quantities:
+                if raise_error:
+                    raise ValidationError(_('Stock item no longer exists'))
+
+                logger.warning(
+                    'Stock item <%s> no longer exists - merge cancelled', item.pk
+                )
+                return
+
+            item.quantity = locked_quantities[item.pk]
+
         user = kwargs.get('user')
         location = kwargs.get('location', self.location)
         notes = kwargs.get('notes') or ''
@@ -2667,6 +2707,10 @@ class StockItem(
         if quantity <= 0:
             return None
 
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            return None
+
         # Also doesn't make sense to split the full amount
         if quantity >= self.quantity:
             return None
@@ -2834,6 +2878,10 @@ class StockItem(
         """
         current_location = self.location
 
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            return False
+
         try:
             quantity = Decimal(kwargs.pop('quantity', self.quantity))
         except InvalidOperation:
@@ -2903,6 +2951,37 @@ class StockItem(
 
         return True
 
+    def lock_quantity(self) -> bool:
+        """Acquire a database row-level lock on this StockItem.
+
+        Once the lock is held, the 'quantity' field is refreshed from the database,
+        so that read-modify-write adjustments operate on the current committed value
+        rather than a (potentially stale) in-memory copy.
+
+        Note: Must be called from within a database transaction,
+        and the lock is held until that transaction completes.
+
+        Returns:
+            False if this StockItem no longer exists in the database.
+        """
+        if not self.pk:
+            return False
+
+        quantity = (
+            StockItem.objects
+            .select_for_update()
+            .filter(pk=self.pk)
+            .values_list('quantity', flat=True)
+            .first()
+        )
+
+        if quantity is None:
+            return False
+
+        self.quantity = quantity
+
+        return True
+
     @transaction.atomic
     def updateQuantity(self, quantity):
         """Update stock quantity for this item.
@@ -2965,6 +3044,10 @@ class StockItem(
         if count < 0:
             return False
 
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            return False
+
         tracking_info = {}
 
         location = kwargs.pop('location', None)
@@ -3035,6 +3118,10 @@ class StockItem(
         if quantity <= 0:
             return False
 
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            return False
+
         tracking_info = {}
 
         status = self._resolve_status_kwarg(kwargs)
@@ -3091,6 +3178,10 @@ class StockItem(
         if quantity <= 0:
             return False
 
+        # Lock the database row, so concurrent adjustments are serialized
+        if not self.lock_quantity():
+            return False
+
         deltas = {}
 
         status = self._resolve_status_kwarg(kwargs)

src/backend/InvenTree/stock/serializers.py

@@ -1993,6 +1993,10 @@ class StockAdjustmentSerializer(serializers.Serializer):
         if len(items) == 0:
             raise ValidationError(_('A list of stock items must be provided'))
 
+        # Process items in stable (pk) order, so that concurrent multi-item
+        # requests acquire database row locks in the same order (deadlock avoidance)
+        data['items'] = sorted(items, key=lambda entry: entry['pk'].pk)
+
         return data
 
 

src/backend/InvenTree/stock/tests.py

@@ -514,6 +514,71 @@ class StockTest(StockTestBase):
         self.assertIn(grandchild, child_1.children.all())
         self.assertNotIn(grandchild, parent.children.all())
 
+    def test_adjustment_stale_quantity(self):
+        """Stock adjustments operate on database quantities, not stale in-memory copies.
+
+        Simulates concurrent adjustment operations, where each worker holds
+        its own (stale) in-memory copy of the same StockItem.
+        """
+        item = StockItem.objects.get(pk=1234)
+        self.assertEqual(item.quantity, 1234)
+
+        # Remove stock via two independent in-memory copies
+        item_a = StockItem.objects.get(pk=item.pk)
+        item_b = StockItem.objects.get(pk=item.pk)
+
+        self.assertTrue(item_a.take_stock(100, self.user))
+        self.assertTrue(item_b.take_stock(200, self.user))
+
+        item.refresh_from_db()
+        self.assertEqual(item.quantity, 934)
+
+        # Add stock via a stale copy
+        item_c = StockItem.objects.get(pk=item.pk)
+        item.take_stock(34, self.user)
+
+        self.assertTrue(item_c.add_stock(100, self.user))
+
+        item.refresh_from_db()
+        self.assertEqual(item.quantity, 1000)
+
+        # Split stock via a stale copy
+        item_d = StockItem.objects.get(pk=item.pk)
+        item.take_stock(500, self.user)
+
+        child = item_d.splitStock(300, None, self.user)
+
+        item.refresh_from_db()
+        self.assertEqual(item.quantity, 200)
+        self.assertEqual(child.quantity, 300)
+
+        # A full-quantity move via a stale copy must not resurrect removed stock
+        item_e = StockItem.objects.get(pk=item.pk)
+        item.take_stock(50, self.user)
+
+        self.assertTrue(item_e.move(self.diningroom, 'Move', self.user))
+
+        item.refresh_from_db()
+        self.assertEqual(item.quantity, 150)
+        self.assertEqual(item.location, self.diningroom)
+
+    def test_merge_stale_quantity(self):
+        """Merging stock items uses database quantities, not stale in-memory values."""
+        part = Part.objects.get(pk=3)
+
+        target = StockItem.objects.create(part=part, quantity=100)
+        source = StockItem.objects.create(part=part, quantity=50)
+
+        # Hold a stale copy of the target, and adjust the real row underneath it
+        stale_target = StockItem.objects.get(pk=target.pk)
+        target.take_stock(60, self.user)
+
+        stale_target.merge_stock_items([source], user=self.user)
+
+        target.refresh_from_db()
+        self.assertEqual(target.quantity, 90)
+        self.assertFalse(StockItem.objects.filter(pk=source.pk).exists())
+
     def test_stocktake(self):
         """Test stocktake function."""
         # Perform stocktake