From fcc68ee69fa6deb38b5f32677f586499f1a62020 Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 14 Jul 2026 12:02:02 +1000 Subject: [PATCH] Apply row-level locking when updating stock item quantity (#12385) --- src/backend/InvenTree/stock/models.py | 91 ++++++++++++++++++++++ src/backend/InvenTree/stock/serializers.py | 4 + src/backend/InvenTree/stock/tests.py | 65 ++++++++++++++++ 3 files changed, 160 insertions(+) diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index 98d7f3b0da..3336c1c8d2 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/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) diff --git a/src/backend/InvenTree/stock/serializers.py b/src/backend/InvenTree/stock/serializers.py index 1d5f4fc029..debd92ddb2 100644 --- a/src/backend/InvenTree/stock/serializers.py +++ b/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 diff --git a/src/backend/InvenTree/stock/tests.py b/src/backend/InvenTree/stock/tests.py index 0d402638b2..dc186aeef3 100644 --- a/src/backend/InvenTree/stock/tests.py +++ b/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