diff --git a/src/controller/testsuite/test_apply_pump.c b/src/controller/testsuite/test_apply_pump.c index c8b3c57a..aa6d1376 100644 --- a/src/controller/testsuite/test_apply_pump.c +++ b/src/controller/testsuite/test_apply_pump.c @@ -240,6 +240,11 @@ static void tap_setup(void) static void tap_teardown(void) { repl_broadcast_send_hook = NULL; + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). */ + persist_stop(&g_tap_c.persist); commit_engine_fini(&g_tap_hce); persist_stop(&g_tap_c.persist); persist_fini(&g_tap_c.persist); @@ -1223,6 +1228,13 @@ static void tpr_setup(int n_vcpus) static void tpr_teardown(void) { repl_broadcast_send_hook = NULL; + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). vtaskmgr_fini() stays AFTER commit_engine_fini() per the + * pre-existing invariant (nothing can dispatch a new vtask onto the + * pool mid-teardown) -- only persist_stop()'s position moves. */ + persist_stop(&g_tpr_c.persist); commit_engine_fini(&g_tpr_hce); vtaskmgr_fini(&g_tpr_pool); persist_stop(&g_tpr_c.persist); diff --git a/src/controller/testsuite/test_apply_pump_integration.c b/src/controller/testsuite/test_apply_pump_integration.c index d0f791fd..cd201ed5 100644 --- a/src/controller/testsuite/test_apply_pump_integration.c +++ b/src/controller/testsuite/test_apply_pump_integration.c @@ -418,6 +418,13 @@ static void tpi_teardown(void) tpi_scan_replies(&g_tpi_req_ep, &drain); /* free any leftover packets */ repl_broadcast_send_hook = NULL; + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). vtaskmgr_fini() stays AFTER commit_engine_fini() per the + * pre-existing invariant noted below -- only persist_stop()'s position + * moves earlier. */ + persist_stop(&g_tpi_c.persist); commit_engine_fini(&g_tpi_hce); if (g_tpi_pool_active) { /* Mirrors test_apply_pump.c's tpr_teardown ordering exactly: diff --git a/src/controller/testsuite/test_commit_barrier_rollout.c b/src/controller/testsuite/test_commit_barrier_rollout.c index 09a3a8a2..90f7a27f 100644 --- a/src/controller/testsuite/test_commit_barrier_rollout.c +++ b/src/controller/testsuite/test_commit_barrier_rollout.c @@ -483,6 +483,13 @@ static void test_fact_hook_append_is_retained_and_nonblocking(void) /* Teardown */ repl_broadcast_send_hook = NULL; + /* bug-1429/bug-1452: persist_stop() must run BEFORE commit_engine_fini() + * -- otherwise the live fsync_worker background thread can still be + * mid-flight inside p->on_durable_advance(ce) (persist.c's fsync_worker) + * while commit_engine_fini() destroys ce->lock/ce->cond, a TOCTOU race + * that hangs pthread_cond_destroy() (live-reproduced, gdb-confirmed). + * The persist_stop() call below becomes a safe, idempotent no-op. */ + persist_stop(g_fp); commit_engine_fini(&g_fce); persist_stop(g_fp); persist_fini(g_fp); @@ -1285,6 +1292,10 @@ static void cb5_teardown(void) cb5_drain_ep(&g_cb5_sb_ep[0]); cb5_drain_ep(&g_cb5_sb_ep[1]); cb5_drain_ep(&g_cb5_req_ep); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * jap_teardown's comment below for the full TOCTOU-race rationale + * (live-reproduced, gdb-confirmed hang in this exact class of fixture). */ + persist_stop(&g_cb5_c.persist); commit_engine_fini(&g_cb5_eng); /* persist_fini calls repl_fini internally (started-persist fixture). */ persist_stop(&g_cb5_c.persist); @@ -1874,6 +1885,23 @@ static void jap_setup(uint32_t sync_timeout_ms) static void jap_teardown(void) { repl_broadcast_send_hook = NULL; + /* bug-1429 (root cause of this session's live, gdb-confirmed hang -- + * thread 1 stuck forever in commit_engine_fini -> pthread_cond_destroy + * at this exact call site, while two fsync_worker threads sat parked in + * pthread_cond_timedwait on this same g_jap): persist_stop() must run + * BEFORE commit_engine_fini() -- exactly the ordering bug-1452 already + * fixed for test_persist_truncate.c and bug-1687 for cbrp_teardown() + * (test_commit_barrier_reply.c). commit_engine_fini() destroys + * g_jace.lock/g_jace.cond; the live fsync_worker background thread + * (started by persist_start(), stopped only by persist_stop()'s + * pthread_join) can still be mid-flight inside p->on_durable_advance(ce) + * (persist.c's fsync_worker) -- a genuine TOCTOU data race with no + * synchronization between the two threads. Calling persist_stop() here + * first guarantees the flusher thread cannot be in-flight at all by the + * time commit_engine_fini() runs. The persist_stop() call below becomes + * a safe, idempotent no-op (fsync_thread_running and wal_fd are already + * cleared by the first call). */ + persist_stop(&g_jap); commit_engine_fini(&g_jace); persist_stop(&g_jap); persist_fini(&g_jap); diff --git a/src/controller/testsuite/test_commit_durability.c b/src/controller/testsuite/test_commit_durability.c index d62a7a47..659ccada 100644 --- a/src/controller/testsuite/test_commit_durability.c +++ b/src/controller/testsuite/test_commit_durability.c @@ -498,8 +498,16 @@ static void test_durable_advance_drives_commit_single_member(void) /* committed never exceeds durable (the invariant the wake preserves). */ TEST_ASSERT_TRUE(atomic_load(&p->committed_lsn) <= persist_durable_lsn(p)); - /* Tear the engine down BEFORE the persist (unhooks the callback so the - * flusher cannot fire against a destroyed engine). */ + /* bug-1429/bug-1452: persist_stop() must run BEFORE commit_engine_fini() + * -- unhooking the callback (persist_set_durable_cb, inside + * commit_engine_fini) alone is NOT sufficient: the flusher thread can + * have already read the OLD, non-NULL callback pointer moments before + * the unhook takes effect and still be mid-flight calling back into the + * engine while commit_engine_fini() destroys ce->lock/ce->cond -- a + * TOCTOU race live-reproduced (gdb-confirmed hang) in the sibling + * test_commit_barrier_rollout.c fixture. teardown_durability_persist's + * own persist_stop() call becomes a safe, idempotent no-op. */ + persist_stop(p); commit_engine_fini(&ce); teardown_durability_persist(p); printf("test_durable_advance_drives_commit_single_member: PASS\n"); @@ -778,6 +786,10 @@ static void test_commit_never_exceeds_durable_invariant(void) TEST_ASSERT_EQUAL_UINT64(tip, committed); TEST_ASSERT_TRUE(atomic_load(&p->committed_lsn) <= persist_durable_lsn(p)); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_durable_advance_drives_commit_single_member above for the full + * TOCTOU-race rationale. */ + persist_stop(p); commit_engine_fini(&ce); teardown_durability_persist(p); printf("test_commit_never_exceeds_durable_invariant: PASS\n"); @@ -967,6 +979,10 @@ static void test_durable_majority_commit_end_to_end(void) /* The invariant the whole spec rests on: committed never exceeds durable. */ TEST_ASSERT_TRUE(atomic_load(&p->committed_lsn) <= persist_durable_lsn(p)); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_durable_advance_drives_commit_single_member above for the full + * TOCTOU-race rationale. */ + persist_stop(p); commit_engine_fini(&ce); repl_remove_standby(&p->repl, &g_dur_eps[0]); repl_remove_standby(&p->repl, &g_dur_eps[1]); diff --git a/src/controller/testsuite/test_commit_engine.c b/src/controller/testsuite/test_commit_engine.c index 9cbf07fb..524e1ff7 100644 --- a/src/controller/testsuite/test_commit_engine.c +++ b/src/controller/testsuite/test_commit_engine.c @@ -1497,6 +1497,12 @@ static void test_sync_append_wired_commits_on_per_entry_majority(void) 1, atomic_load(&g_sp->committed_lsn), "committed_lsn must advance to the majority-acked entry"); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). sync_persist_teardown()'s own persist_stop() call becomes a + * safe, idempotent no-op. */ + persist_stop(g_sp); commit_engine_fini(&g_ce); sync_persist_teardown(g_sp); g_sp = NULL; @@ -1541,6 +1547,12 @@ static void test_sync_append_wired_stale_acks_time_out(void) TEST_ASSERT_TRUE_MESSAGE(atomic_load(&g_sp->committed_lsn) < 2, "committed_lsn must stay unmoved past the entry on timeout"); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). sync_persist_teardown()'s own persist_stop() call becomes a + * safe, idempotent no-op. */ + persist_stop(g_sp); commit_engine_fini(&g_ce); sync_persist_teardown(g_sp); g_sp = NULL; @@ -1575,6 +1587,12 @@ static void test_sync_append_wired_single_member_commits_locally(void) 1, atomic_load(&g_sp->committed_lsn), "n_peers=1 sync append must advance committed_lsn to the entry"); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). sync_persist_teardown()'s own persist_stop() call becomes a + * safe, idempotent no-op. */ + persist_stop(g_sp); commit_engine_fini(&g_ce); sync_persist_teardown(g_sp); g_sp = NULL; diff --git a/src/controller/testsuite/test_convergence_verification.c b/src/controller/testsuite/test_convergence_verification.c index 5670e9b0..5e7f49ef 100644 --- a/src/controller/testsuite/test_convergence_verification.c +++ b/src/controller/testsuite/test_convergence_verification.c @@ -208,6 +208,12 @@ static void cv_teardown(void) { repl_broadcast_send_hook = NULL; ha_repl_stream_wal_hook = NULL; + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture: a live fsync_worker thread can still call back into the + * commit engine while commit_engine_fini() destroys its lock/cond). */ + persist_stop(&g_cv.persist); commit_engine_fini(&g_cv.commit); delaymgr_fini(&g_cv.delaymgr); persist_stop(&g_cv.persist); diff --git a/src/controller/testsuite/test_demotion_convergence.c b/src/controller/testsuite/test_demotion_convergence.c index 37382d34..41bb39f5 100644 --- a/src/controller/testsuite/test_demotion_convergence.c +++ b/src/controller/testsuite/test_demotion_convergence.c @@ -230,6 +230,15 @@ static void dc_teardown(void) ha_restart_hook = NULL; ha_fatal_hook = NULL; slowlane_fini(&g_dc.slowlane); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- this + * exact site was flagged, but left unfixed, as a follow-up by bug-1452's + * own remediation note ("the identical commit_engine_fini-before- + * persist_stop ordering also exists in test_demotion_convergence.c... + * same latent race, not yet observed to crash there"). Fixed now as + * part of the bug-1429 sweep after a live, gdb-confirmed hang in the + * sibling test_commit_barrier_rollout.c fixture proved this class of + * race is a genuine, reproducible controller_unit_test hang source. */ + persist_stop(&g_dc.persist); commit_engine_fini(&g_dc.commit); delaymgr_fini(&g_dc.delaymgr); persist_stop(&g_dc.persist); diff --git a/src/controller/testsuite/test_egress_filter.c b/src/controller/testsuite/test_egress_filter.c index 10091333..581bd03c 100644 --- a/src/controller/testsuite/test_egress_filter.c +++ b/src/controller/testsuite/test_egress_filter.c @@ -217,6 +217,11 @@ static void ef_setup(void) static void ef_teardown(void) { repl_broadcast_send_hook = NULL; + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). */ + persist_stop(&g_c.persist); commit_engine_fini(&g_ce); persist_stop(&g_c.persist); persist_fini(&g_c.persist); diff --git a/src/controller/testsuite/test_ha_role_lifecycle.c b/src/controller/testsuite/test_ha_role_lifecycle.c index 24be531c..94003a4f 100644 --- a/src/controller/testsuite/test_ha_role_lifecycle.c +++ b/src/controller/testsuite/test_ha_role_lifecycle.c @@ -691,6 +691,11 @@ static void lifecycle_cluster_teardown(void) #ifdef XCE_TESTING repl_broadcast_send_hook = NULL; #endif + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). */ + persist_stop(&g_lc.persist); commit_engine_fini(&g_lc.commit); delaymgr_fini(&g_lc.delaymgr); persist_stop(&g_lc.persist); diff --git a/src/controller/testsuite/test_job_array_fence.c b/src/controller/testsuite/test_job_array_fence.c index d0a5b6bf..67c65954 100644 --- a/src/controller/testsuite/test_job_array_fence.c +++ b/src/controller/testsuite/test_job_array_fence.c @@ -222,6 +222,11 @@ static void tf_teardown(void) ha_fini(&g_tf_c.ha); repl_broadcast_send_hook = NULL; + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). */ + persist_stop(&g_tf_c.persist); commit_engine_fini(&g_tf_hce); persist_stop(&g_tf_c.persist); persist_fini(&g_tf_c.persist); diff --git a/src/controller/testsuite/test_job_array_replay_fence.c b/src/controller/testsuite/test_job_array_replay_fence.c index 0b5ee472..9480db93 100644 --- a/src/controller/testsuite/test_job_array_replay_fence.c +++ b/src/controller/testsuite/test_job_array_replay_fence.c @@ -813,6 +813,11 @@ static void test_first_boot_promotion_never_truncates_own_durable_wal(void) delaymgr_fini(&g_trf_c.delaymgr); ha_fini(&g_trf_c.ha); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). */ + persist_stop(&g_trf_c.persist); commit_engine_fini(&g_trf_c.commit); persist_stop(&g_trf_c.persist); persist_fini(&g_trf_c.persist); diff --git a/src/controller/testsuite/test_p2_integration.c b/src/controller/testsuite/test_p2_integration.c index 03a71078..5f11539c 100644 --- a/src/controller/testsuite/test_p2_integration.c +++ b/src/controller/testsuite/test_p2_integration.c @@ -277,6 +277,12 @@ static void test_single_node_committed_lsn_advances(void) /* WAL LSN did advance (local write succeeded) */ TEST_ASSERT_TRUE(p->current_lsn > 0); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_commit_barrier_rollout.c's jap_teardown for the full TOCTOU-race + * rationale (live-reproduced, gdb-confirmed hang in this exact class of + * fixture). teardown_p2_persist()'s own persist_stop() call becomes a + * safe, idempotent no-op. */ + persist_stop(p); commit_engine_fini(&ce); teardown_p2_persist(p); printf("test_single_node_committed_lsn_advances: PASS\n"); @@ -344,6 +350,10 @@ static void test_committed_lsn_does_not_advance_on_timeout(void) TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, committed_after, "committed_lsn must stay unmoved when the entry never reaches majority"); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_single_node_committed_lsn_advances above for the full TOCTOU-race + * rationale. */ + persist_stop(p); commit_engine_fini(&ce); teardown_p2_persist(p); printf("test_committed_lsn_does_not_advance_on_timeout: PASS\n"); @@ -448,6 +458,10 @@ static void test_committed_lsn_advances_after_majority_ack(void) committed_after > committed_before || committed_after == written_lsn, "committed_lsn must advance after majority ACK (Req 4.1)"); + /* bug-1429/bug-1452: persist_stop() before commit_engine_fini() -- see + * test_single_node_committed_lsn_advances above for the full TOCTOU-race + * rationale. */ + persist_stop(g_p9); commit_engine_fini(&ce); teardown_p2_persist(g_p9); g_p9 = NULL;