Repository navigation
test(daemon-ipc): wait for the startup gate event, not an attempt budget - #2409
Merged
Merged
Conversation
daemon_ipc_windows_startup_retries_transient_rendezvous_reader failed on a windows-latest CLANG64 runner (ASSERT(startup_observed); the whole suite took 7 s) on a PR that touched only CLI files. ipc_test_startup_gate_wait_acquired polled 50000 x cbm_usleep(200) and returned false once the budget ran out. On POSIX that is ~10 s, but on Windows cbm_usleep(us) is Sleep(us / 1000), so cbm_usleep(200) is Sleep(0): the budget collapsed to 50000 bare yields, which a loaded runner spends in milliseconds before the startup thread reaches the gate. A timeout decided the verdict. The helper now waits until the startup thread has either parked in the gate or returned without reaching it (a new `finished` flag, set by ipc_test_win_startup_call after its call returns). Both flags only go false -> true and one of them is always set: before the gate the startup path only try-acquires, and a thread parked in the gate cannot return until the test releases it. No bound is left to decide the verdict. A thread that returns without reaching the gate fails at once, and the test reports its try-acquire status as the reason. The same shape is removed from two cross-platform tests: - daemon_ipc_wait_forever_is_interruptible waited for the server's forever-wait under a 2 s wall-clock deadline. Past the deadline the test skipped the interrupt, so the join then blocked on a server parked in WAIT_FOREVER: a hang, not a failure. It now waits for `waiting` or for a new `finished` flag set when the server thread returns. - daemon_ipc_connect_waits_for_delayed_listener capped its wait for `delay_started` at 2000 polls. That store is the thread's first action, so the wait needs no attempt bound. No production change and no new sleep: the existing per-poll yields stay. Proof. A standalone harness mirrors the gate, both helpers and the thread wrapper, with cbm_usleep modelled as on Windows (Sleep(us / 1000), so cbm_usleep(200) is a bare yield): - startup thread arriving only after 50000 polls: old helper 20/20 FAIL (startup_observed=false with call.result=1, the CI signature), new helper 20/20 PASS; - startup thread descheduled 500 ms: the old helper gives up after 9 ms, 10/10 FAIL; new helper 10/10 PASS; control, old helper with POSIX usleep: 10/10 PASS; - thread returning without reaching the gate: the new helper fails within 11 ms with call.result=-1 (the old one took 16 s under POSIX). Real daemon_ipc suite on macOS: with the server and listener threads held 4 s and 6 s (beyond both old bounds) the two cross-platform tests pass. The old 2000-poll loop under the 6 s hold fails ASSERT(delay_observed). The old 2 s deadline under the 4 s hold hangs in the join until killed. A server that skips accept makes the new loop fail ASSERT(reached_wait) with no added wait. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three waits in
tests/test_daemon_ipc.clet a bound decide the verdict. Each now waits for its event instead: either the thing it waits for happens, or the thread returns without it.ipc_test_startup_gate_wait_acquired(both Windows rendezvous tests) now waits foracquiredOR a newfinishedflag set byipc_test_win_startup_call. A thread that returns without reaching the gate fails immediately and reports the startup call's return value.daemon_ipc_wait_forever_is_interruptible: the 2 s wall-clock deadline is replaced by a wait forwaitingORfinished.daemon_ipc_connect_waits_for_delayed_listener: the 2000-poll cap is removed.delay_startedis the thread's first store, so the wait needs no cap.No production change and no new sleep.
Why
On Windows,
cbm_usleep(us)isSleep(us / 1000). The gate helper's50000 x cbm_usleep(200)budget was meant to be about 10 s, but it is really 50000 ×Sleep(0): bare yields that a loaded runner can spend in milliseconds. That is howdaemon_ipc_windows_startup_retries_transient_rendezvous_readerfailedASSERT(startup_observed)on windows-latest CLANG64 (run 36354336229, job 108779549114; the whole suite took 7 s).The old
wait_foreverdeadline was worse than a flake. Once it expired, the test skipped the interrupt and hung in the join.Evidence
call.result=1, the CI signature); the new one passes 20/20.usleep: passes 10/10.daemon_ipcwith threads held 4 s and 6 s:ASSERT(delay_observed).daemon_ipc: 53 passed, 2 platform skips, 4.0 s.lint-ciis clean.-Werror) is clean.Found attributing a Windows CI red on #1781.