From 4c6ecffdcdb8d78c0704f0d7e825f676ad77c240 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 28 Sep 2026 19:43:10 +0200 Subject: [PATCH] test(daemon-ipc): wait for the startup gate event, not an attempt budget 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 --- tests/test_daemon_ipc.c | 68 +++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index 815f630a6..0e212a8d6 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -401,6 +401,7 @@ static int ipc_test_win_lock_child(const char *kind, const char *key, const char typedef struct { const cbm_daemon_ipc_endpoint_t *endpoint; int result; + atomic_bool finished; /* set once the startup call has fully returned */ } ipc_test_win_startup_call_t; typedef struct { @@ -421,16 +422,22 @@ static void ipc_test_startup_gate(void *context) { } } -static bool ipc_test_startup_gate_wait_acquired(ipc_test_startup_gate_t *gate) { - /* `acquired` never clears, so this wait cannot miss the event; the bound - * only catches a thread that never started at all. */ - for (size_t attempt = 0; attempt < 50000U; attempt++) { - if (atomic_load(&gate->acquired)) { - return true; - } +/* Waits until the startup thread has either parked in the gate or returned + * without reaching it. Both flags only ever go false -> true, and one of them + * is always eventually set: before the gate the startup path only try-acquires + * (no blocking wait on anything this test holds), and a thread parked in the + * gate cannot return until the test releases it. So no attempt or time bound + * is needed, and none may decide the verdict: the previous 50000 x + * cbm_usleep(200) budget meant ~10 s on POSIX but, since cbm_usleep(200) is + * Sleep(0) on Windows, only 50000 bare yields there -- a loaded CI runner + * exhausted them before the thread reached the gate. A thread that never + * started is caught by the caller's thread_started check. */ +static bool ipc_test_startup_gate_wait_acquired(ipc_test_startup_gate_t *gate, + ipc_test_win_startup_call_t *call) { + while (!atomic_load(&gate->acquired) && !atomic_load(&call->finished)) { cbm_usleep(200); } - return false; + return atomic_load(&gate->acquired); } static void *ipc_test_win_startup_call(void *opaque) { @@ -441,6 +448,7 @@ static void *ipc_test_win_startup_call(void *opaque) { call->result = 0; } cbm_daemon_ipc_startup_lock_release(&startup); + atomic_store(&call->finished, true); return NULL; } @@ -1395,6 +1403,7 @@ TEST(daemon_ipc_windows_startup_retries_transient_rendezvous_reader) { bool startup_observed = false; int join_status = -1; ipc_test_win_startup_call_t call = {.result = -1}; + atomic_init(&call.finished, false); ipc_test_startup_gate_t gate; atomic_init(&gate.acquired, false); atomic_init(&gate.may_proceed, false); @@ -1420,7 +1429,7 @@ TEST(daemon_ipc_windows_startup_retries_transient_rendezvous_reader) { thread_started = cbm_thread_create(&thread, 0, ipc_test_win_startup_call, &call) == 0; } if (thread_started) { - startup_observed = ipc_test_startup_gate_wait_acquired(&gate); + startup_observed = ipc_test_startup_gate_wait_acquired(&gate, &call); } ipc_test_win_lock_release(&record_reader); atomic_store(&gate.may_proceed, true); @@ -1441,7 +1450,14 @@ TEST(daemon_ipc_windows_startup_retries_transient_rendezvous_reader) { ASSERT_EQ(directory_status, CBM_PRIVATE_FILE_LOCK_OK); ASSERT_EQ(record_status, CBM_PRIVATE_FILE_LOCK_OK); ASSERT_TRUE(thread_started); - ASSERT_TRUE(startup_observed); + if (!startup_observed) { + /* The startup thread returned without reaching the gate; its + * try-acquire status is the true reason. */ + char reason[96]; + (void)snprintf(reason, sizeof(reason), "startup call returned %d without reaching the gate", + call.result); + FAIL(reason); + } ASSERT_EQ(join_status, 0); ASSERT_EQ(call.result, 1); ASSERT_TRUE(address[0] != '\0'); @@ -1464,6 +1480,7 @@ TEST(daemon_ipc_windows_rendezvous_bridges_concurrent_lifetime_owner) { bool startup_observed = false; int join_status = -1; ipc_test_win_startup_call_t call = {.result = -1}; + atomic_init(&call.finished, false); bool parent_ok = ipc_test_parent_new(parent, "win-lifetime-bridge"); if (parent_ok) { @@ -1501,7 +1518,7 @@ TEST(daemon_ipc_windows_rendezvous_bridges_concurrent_lifetime_owner) { thread_started = cbm_thread_create(&thread, 0, ipc_test_win_startup_call, &call) == 0; } if (thread_started) { - startup_observed = ipc_test_startup_gate_wait_acquired(&gate); + startup_observed = ipc_test_startup_gate_wait_acquired(&gate, &call); } /* Claim the lifetime lock while startup is parked in the gate, so the * concurrent-owner bridge under test is exercised deterministically. */ @@ -1534,7 +1551,14 @@ TEST(daemon_ipc_windows_rendezvous_bridges_concurrent_lifetime_owner) { ASSERT_EQ(directory_status, CBM_PRIVATE_FILE_LOCK_OK); ASSERT_EQ(record_status, CBM_PRIVATE_FILE_LOCK_OK); ASSERT_TRUE(thread_started); - ASSERT_TRUE(startup_observed); + if (!startup_observed) { + /* The startup thread returned without reaching the gate; its + * try-acquire status is the true reason. */ + char reason[96]; + (void)snprintf(reason, sizeof(reason), "startup call returned %d without reaching the gate", + call.result); + FAIL(reason); + } ASSERT_EQ(lifetime_status, CBM_PRIVATE_FILE_LOCK_OK); ASSERT_EQ(join_status, 0); ASSERT_EQ(call.result, 0); @@ -2442,6 +2466,7 @@ typedef struct { cbm_daemon_ipc_listener_t *listener; cbm_daemon_ipc_connection_t *connection; atomic_bool waiting; + atomic_bool finished; /* set once the server thread has returned */ int receive_result; } ipc_forever_wait_server_t; @@ -2464,6 +2489,7 @@ static void *ipc_forever_wait_server(void *opaque) { server->connection, CBM_DAEMON_IPC_WAIT_FOREVER, &frame, &payload); } free(payload); + atomic_store_explicit(&server->finished, true, memory_order_release); return NULL; } @@ -2480,6 +2506,7 @@ TEST(daemon_ipc_wait_forever_is_interruptible) { int join_result = -1; ipc_forever_wait_server_t server = {0}; atomic_init(&server.waiting, false); + atomic_init(&server.finished, false); if (ipc_test_parent_new(parent, "wait-forever")) { endpoint = cbm_daemon_ipc_endpoint_new(key, parent); @@ -2495,9 +2522,11 @@ TEST(daemon_ipc_wait_forever_is_interruptible) { if (thread_started) { client = cbm_daemon_ipc_connect(endpoint, 2000); } - uint64_t deadline = cbm_now_ms() + 2000; - while (client && cbm_now_ms() < deadline && - !atomic_load_explicit(&server.waiting, memory_order_acquire)) { + /* Wait for the event itself, never a deadline: the server either enters + * its forever-wait or returns (its accept is bounded by the call's own + * timeout), and both flags only ever go false -> true. */ + while (client && !atomic_load_explicit(&server.waiting, memory_order_acquire) && + !atomic_load_explicit(&server.finished, memory_order_acquire)) { cbm_usleep(1000); } reached_wait = atomic_load_explicit(&server.waiting, memory_order_acquire); @@ -2561,14 +2590,13 @@ TEST(daemon_ipc_connect_waits_for_delayed_listener) { thread_started = cbm_thread_create(&thread, 0, ipc_delayed_listener_start, &delayed) == 0; } if (thread_started) { + /* `delay_started` is the thread's first store, so it cannot return + * without setting it: wait for that event with no attempt bound. */ struct timespec poll_delay = {.tv_sec = 0, .tv_nsec = 1000000}; - for (size_t i = 0; i < 2000; i++) { - if (atomic_load_explicit(&delayed.delay_started, memory_order_acquire)) { - delay_observed = true; - break; - } + while (!atomic_load_explicit(&delayed.delay_started, memory_order_acquire)) { (void)cbm_nanosleep(&poll_delay, NULL); } + delay_observed = true; } if (delay_observed) { client = cbm_daemon_ipc_connect(endpoint, 1500);