From 8cefd1a5465e838969628b5b87284435a3f79a5e Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 22 Dec 2018 21:06:39 +0000 Subject: [PATCH] Have transactions verify/assert safe usage Have the transaction code verify that there is at most one active child at the time and no one is using the parent while child is active. This is how the code is intended to be used and also the code almost certainly does not work otherwise. The new code does not cover commiting/rolling back a parent before a child but that is already covered by the existing code (it will trigger when child transaction is rolled back/committed or when leaving the contextmanager from start_transaction). This would have caught 7d758760d1c08bcd3b364f7e6ec76b2c0eafe68a immediately with an assertion error. Signed-off-by: Niels Thykier --- britney2/transaction.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/britney2/transaction.py b/britney2/transaction.py index e26d53a..a5d7efc 100644 --- a/britney2/transaction.py +++ b/britney2/transaction.py @@ -7,8 +7,16 @@ class MigrationTransactionState(object): self._is_rolled_back = False self._is_committed = False self._undo_items = [] + self._pending_child = False + if self.parent_transaction: + # Transactions can only support one child transaction at a time + assert not self.parent_transaction._pending_child + self.parent_transaction._pending_child = True def add_undo_item(self, undo, updated_binaries): + # We do not accept any changes to this transaction while it has a child transaction + # (the undo code does not handle that case correctly) + assert not self._pending_child self._assert_open_transaction() self._undo_items.append((undo, updated_binaries)) @@ -33,6 +41,7 @@ class MigrationTransactionState(object): self._assert_open_transaction() self._is_committed = True if self.parent_transaction: + self.parent_transaction._pending_child = False for undo_item in self._undo_items: self.parent_transaction.add_undo_item(*undo_item) @@ -43,7 +52,6 @@ class MigrationTransactionState(object): """ self._assert_open_transaction() - self._is_rolled_back = True lundo = self._undo_items lundo.reverse() @@ -108,6 +116,9 @@ class MigrationTransactionState(object): else: provides_t[arch][provided_pkg] = undo['virtual'][p] + if self.parent_transaction: + self.parent_transaction._pending_child = False + @property def is_rolled_back(self): return self._is_rolled_back