Skip to content

Commit

Permalink
model.base: fix ConstrainedList.clear() atomicity
Browse files Browse the repository at this point in the history
The default inherited `clear()` implementation repeatedly deletes the
last item of the list until the list is empty. If the last item can be
deleted successfully, but an item in front of it that will be deleted
later cannot, this makes `clear()` non-atomic.

Thus, the `clear()` method is now overriden in an atomic way.
Furthermore, the ConstrainedList atomicity test is fixed to correctly
test for this as well.
  • Loading branch information
jkhsjdhjs authored and s-heppner committed Nov 13, 2023
1 parent 5eb895e commit 886313e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
4 changes: 4 additions & 0 deletions basyx/aas/model/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,10 @@ def extend(self, values: Iterable[_T]) -> None:
self._item_add_hook(v, self._list + v_list[:idx])
self._list = self._list + v_list

def clear(self) -> None:
# clear() repeatedly deletes the last item by default, making it not atomic
del self[:]

@overload
def __getitem__(self, index: int) -> _T: ...

Expand Down
10 changes: 9 additions & 1 deletion test/model/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,15 @@ def hook(itm: int, _list: List[int]) -> None:
self.assertEqual(c_list, [1, 2, 3])
with self.assertRaises(ValueError):
c_list.clear()
self.assertEqual(c_list, [1, 2, 3])
# the default clear() implementation seems to repeatedly delete the last item until the list is empty
# in this case, the last item is 3, which cannot be deleted because it is > 2, thus leaving it unclear whether
# clear() really is atomic. to work around this, the list is reversed, making 1 the last item, and attempting
# to clear again.
c_list.reverse()
with self.assertRaises(ValueError):
c_list.clear()
self.assertEqual(c_list, [3, 2, 1])
c_list.reverse()
del c_list[0:2]
self.assertEqual(c_list, [3])

Expand Down

0 comments on commit 886313e

Please sign in to comment.