Skip to content

test(utils): wait for the read only lock to register before probing - #4943

Draft
colinbarry wants to merge 3 commits into
masterfrom
fix/flaky-prioritise-readonly-lock
Draft

colinbarry wants to merge 3 commits into
masterfrom
fix/flaky-prioritise-readonly-lock

Conversation

@colinbarry

@colinbarry colinbarry commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

ResourceLockTest.PrioritiseReadOnlyLock slept 15ms after its latch, hoping the deferred thread had reached lock() and registered as pending, then probed once with a WRITE try_lock. The latch only says the thread is about to call lock(), so under CPU load the probe lands first, sees a gate that is legitimately still open, acquires, and fails the test. The failure then hung the binary rather than reporting: the assertion returned from the test body while the deferred thread was still parked in lock(), and only the release the test never reached could free it, so ~future blocked until the job timeout.

The probe now retries until the gate is observably closed, and the release happens before the result is reported. A WRITE refused there can only be the pending READ_ONLY, since the WRITE the test already holds is compatible with another WRITE. The worker also asks for std::launch::async explicitly; without it a deferred launch runs nothing until get(), and the test blocks on a latch whose other arrival never comes.

Closes #4941

@colinbarry colinbarry self-assigned this Sep 25, 2026
@colinbarry colinbarry added the bug bug label Sep 25, 2026
@colinbarry

Copy link
Copy Markdown
Contributor Author

Tracking

  • [Link to Epic/Issue]

Standard development

CI Testing Labels

  • Select the appropriate CI test labels (CI -build=build-name -test=test-suite)

Documentation checklist

  • Add the documentation label
  • Add the bug / feature label
  • Add the milestone for which this feature is intended
    • If not known, set for a later milestone
  • Write a release note, including added/changed clauses
    • What has changed? What does it mean for a user? What should a user do with it? [#{{PR_number}}]({{link to the PR}})
  • [ Documentation PR link memgraph/documentation#XXXX ]
    • Is back linked to this development PR

@colinbarry
colinbarry force-pushed the fix/flaky-prioritise-readonly-lock branch from 433c5af to 154c0cd Compare September 29, 2026 11:19
@github-actions

Copy link
Copy Markdown
Contributor

This PR has potential conflicts with the following other open pull requests which modify the same files:

@colinbarry
colinbarry force-pushed the fix/flaky-prioritise-readonly-lock branch 2 times, most recently from f34e865 to 3c24d4c Compare September 29, 2026 11:24
@colinbarry colinbarry added CI -build=release -test=core Run release build and core tests on push Docs unnecessary Docs unnecessary flakiness labels Sep 29, 2026
PrioritiseReadOnlyLock slept 15ms after its latch, hoping the deferred
thread had reached lock() and registered as pending, then probed once
with a WRITE try_lock. Under CPU load the probe lands first, sees a
gate that is legitimately still open, acquires, and fails the test.

The probe now retries until the gate is observably closed. A WRITE
refused here can only be the pending READ_ONLY, since the WRITE the
test already holds is compatible with another WRITE.

The failure also hung the binary: the assertion returned from the test
body while the deferred thread was still parked in lock(), and only
the release the test never reached could free it, so ~future blocked
until the job timeout. The release now happens before the result is
reported.

The worker also asks for std::launch::async explicitly. Without it a
deferred launch runs nothing until get(), and the test blocks on a
latch whose other arrival never comes.

Closes #4941
@colinbarry
colinbarry force-pushed the fix/flaky-prioritise-readonly-lock branch from 3c24d4c to 912e717 Compare September 29, 2026 13:14
The retry loop waited up to 60s for the deferred thread to register as
pending. The whole binary shares a 120s ctest budget across 37 tests, so
a broken lock spent half of it spinning before reporting. The wait is one
mutex acquisition, and 5s matches the watchdog bounds the rest of the file
already uses.
The worker's early-return checks ran before it reached the latch, so a
broken lock left the main thread waiting at the latch forever. The
worker now arrives first, and the failure message names both causes of
an ungated WRITE.
@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug bug CI -build=release -test=core Run release build and core tests on push Docs unnecessary Docs unnecessary flakiness

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: ResourceLockTest.PrioritiseReadOnlyLock fails then hangs under CPU load

1 participant