diff --git a/amber/src/main/python/core/util/atomic.py b/amber/src/main/python/core/util/atomic.py index a73619489fd..55fecb0f459 100644 --- a/amber/src/main/python/core/util/atomic.py +++ b/amber/src/main/python/core/util/atomic.py @@ -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 diff --git a/amber/src/test/python/core/util/test_atomic.py b/amber/src/test/python/core/util/test_atomic.py index 0f6b393233c..824b25b6719 100644 --- a/amber/src/test/python/core/util/test_atomic.py +++ b/amber/src/test/python/core/util/test_atomic.py @@ -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() @@ -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)