Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion amber/src/main/python/core/util/atomic.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ def value(self, v):

def get_and_set(self, v):
with self._lock:
old_value = self.value
old_value = self._value
self._value = int(v)
return old_value
26 changes: 3 additions & 23 deletions amber/src/test/python/core/util/test_atomic.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,7 @@ def test_value_setter_replaces_state_with_int_coercion(self):
a.value = "100"
assert a.value == 100

def test_get_and_set_currently_deadlocks_on_non_reentrant_lock(self):
# Bug pin: get_and_set acquires self._lock and then reads self.value,
# which is a property that ALSO tries to acquire self._lock. The lock
# is a non-reentrant threading.Lock, so the call deadlocks the moment
# it is invoked. Document via thread + timeout so the test surfaces
# the deadlock without hanging the whole suite, and pair it with an
# xfail-strict test below that asserts the intended contract.
def test_get_and_set_does_not_deadlock_on_non_reentrant_lock(self):
a = AtomicInteger(10)
started = threading.Event()
completed = threading.Event()
Expand All @@ -96,23 +90,9 @@ def attempt():
assert not errors, (
f"get_and_set raised before reaching the deadlock spin: {errors[0]!r}"
)
assert worker.is_alive(), (
"worker thread exited unexpectedly — get_and_set neither deadlocked "
"nor completed; the test no longer pins the documented bug."
)
assert not completed.is_set(), (
"get_and_set unexpectedly returned — the deadlock bug appears fixed; "
"delete this pinned test along with the xfail below."
)
assert not worker.is_alive()
assert completed.is_set()

@pytest.mark.xfail(
strict=True,
reason=(
"Known bug: AtomicInteger.get_and_set deadlocks because it holds "
"the non-reentrant lock while accessing the value property. "
"This xfail flips to XPASS when the bug is fixed."
),
)
@pytest.mark.timeout(2)
def test_get_and_set_should_return_old_value_and_replace_state(self):
a = AtomicInteger(10)
Expand Down