Skip to content

Flaky pyamber test: AtomicInteger get_and_set deadlock test races on thread teardown #7294

Description

@mengw15

What happened?

test_get_and_set_does_not_deadlock_on_non_reentrant_lock (amber/src/test/python/core/util/test_atomic.py:68, added in #5010) intermittently fails the build / pyamber (ubuntu-latest, 3.13) job with assert not True. It is a flake: a re-run of the same commit passed.

Failing run https://github.com/apache/texera/actions/runs/30849404497/job/91808158924 (attempt 1)
Head SHA eddb1f63ad0317da93b1c978272d55e5a607b42c (an unrelated storage PR — the diff touches no Python)
Outcome 1 failed / 1013 passed; re-run of the same commit passed

The test starts a worker thread that calls AtomicInteger.get_and_set(99), waits on an Event, then asserts the thread is gone:

def attempt():
    started.set()
    try:
        a.get_and_set(99)
        completed.set()      # set INSIDE the thread, before it exits
    except BaseException as exc:
        errors.append(exc)
...
completed.wait(timeout=0.5)
assert not worker.is_alive()

Two independent wall-clock races make this fail on a healthy implementation:

  1. completed is set by the worker before attempt() returns, so wait() can succeed while the thread is still tearing down. is_alive() is only guaranteed false after a join — any preemption in that window fails the assert, no matter how long the timeout is.
  2. If the worker doesn't finish within the fixed 0.5s budget at all (loaded runner, many parallel jobs), wait() returns False and the assert fails outright.

Expected: the test fails only when get_and_set actually deadlocks (the #4794 regression it guards). Neither race has anything to do with that.

How to reproduce?

Not deterministically reproducible — it is a scheduling race that surfaces under CI load. It fired on the Run pyamber tests step of the build / pyamber (ubuntu-latest, 3.13) leg in the run linked above; re-running the same commit passed.

Fix direction: synchronize on the thread itself instead of a fixed window — worker.join(timeout=<n>) in place of completed.wait(timeout=0.5). That closes race 1 (join returns only once the thread is really dead) and race 2 (a correct implementation joins in microseconds, so a generous timeout costs nothing), while a real deadlock still keeps the worker alive past the timeout and trips the same assertion.

Version/Branch

1.3.0-incubating-SNAPSHOT (main)

What browsers are you seeing the problem on?

No response

Relevant log output

self = <src.test.python.core.util.test_atomic.TestAtomicIntegerSingleThreaded object at 0x7f8d132a8550>
        assert not errors, (
>       assert not worker.is_alive()
E       assert not True
src/test/python/core/util/test_atomic.py:93: AssertionError
FAILED src/test/python/core/util/test_atomic.py::TestAtomicIntegerSingleThreaded::test_get_and_set_does_not_deadlock_on_non_reentrant_lock - assert not True
===== 1 failed, 1013 passed, 1 deselected, 1 xfailed, 4 warnings in 48.63s =====

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions