From d438742f1a36dc6b2a8d23a2445a46b4f9d2640e Mon Sep 17 00:00:00 2001
From: Rongsong Shen <rshen@shenrs.eu>
Date: Thu, 30 Apr 2026 06:48:01 +0000
Subject: [PATCH] fix: staging_merge_complete bootstrap deadlock and missing
 recompact_node_ids
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two correctness fixes found during final Phase 6 code review:

F1 (CRITICAL): GC permanently disabled — staging_merge_complete bootstrapping deadlock.
sched_begin_session reset staging_merge_complete to 0 but never set it back to 1,
so sched_end_session's guard (!staging_merge_complete) always fired, preventing GC
from ever running. select-node memory leaked indefinitely across scheduling sessions.
Fix: set staging_merge_complete=1 at the end of sched_begin_session after all
staging work (sched_account_jobs etc.) completes.

F2 (HIGH): recompact_node_ids() omitted from GC protocol in sched_end_session.
The protocol in resreq.h requires gc_sweep → recompact_node_ids. Without
recompaction, sel_node_next_id grows monotonically across sessions and
select_eval_ctx_t arrays (sized by max node_id) grow without bound.
Fix: call recompact_node_ids() after gc_sweep_select_nodes.

Also adds test_staging_merge_complete_gc_guard to document the bootstrap
invariant: sched_begin_session must leave staging_merge_complete=1 so the
GC guard in sched_end_session does not fire.
---
 src/scheduler/src/scheduler.c  |  3 +++
 src/scheduler/testsuite/test.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c
index aa883d4a..6293405b 100644
--- a/src/scheduler/src/scheduler.c
+++ b/src/scheduler/src/scheduler.c
@@ -770,6 +770,8 @@ void sched_begin_session(xce_scheduler_t* sched)
 
     sched_account_jobs(sched);
 
+    /* Staging merge done — sched_end_session's GC guard reads this field. */
+    atomic_store_explicit(&sched->staging_merge_complete, 1, memory_order_release);
     return;
 }
 
@@ -875,6 +877,7 @@ void sched_end_session(xce_scheduler_t* sched)
     atomic_thread_fence(memory_order_seq_cst);
 
     gc_sweep_select_nodes(NULL, 0);
+    recompact_node_ids();
     free(live_jobs);
 
     atomic_store_explicit(&sched->staging_merge_complete, 1, memory_order_release);
diff --git a/src/scheduler/testsuite/test.c b/src/scheduler/testsuite/test.c
index 602163c2..69eb6ee0 100644
--- a/src/scheduler/testsuite/test.c
+++ b/src/scheduler/testsuite/test.c
@@ -221,6 +221,38 @@ test_gc_predicates_idle_invariant(void)
     sel_optimize_test_reset();
 }
 
+static void
+test_staging_merge_complete_gc_guard(void)
+{
+    /* Regression: sched_begin_session must leave staging_merge_complete=1 so
+     * sched_end_session's guard (!staging_merge_complete) does not fire.
+     *
+     * sched_begin_session: sets staging_merge_complete=0, does all session setup,
+     * then sets staging_merge_complete=1 (staging done).
+     * sched_end_session: guard skips GC when staging_merge_complete==0.
+     *
+     * Without the trailing set-to-1 in sched_begin_session, staging_merge_complete
+     * stays 0 forever and GC never runs — select-node memory leaks indefinitely. */
+    xce_scheduler_t sched;
+    memset(&sched, 0, sizeof(sched));
+
+    /* Simulate the post-begin_session state the fix produces */
+    atomic_store(&sched.session_active, 0);        /* session ended */
+    atomic_store(&sched.eval_in_progress, 0);
+    atomic_store(&sched.staging_merge_complete, 1); /* fix: begin_session sets this */
+
+    /* The GC guard in sched_end_session: must NOT fire */
+    int guard_fires = atomic_load(&sched.eval_in_progress)
+                      || !atomic_load(&sched.staging_merge_complete);
+    TEST_ASSERT_EQUAL_INT(0, guard_fires);
+
+    /* Confirm the unfixed state (staging=0) would block GC */
+    atomic_store(&sched.staging_merge_complete, 0);
+    int guard_fires_unfixed = atomic_load(&sched.eval_in_progress)
+                              || !atomic_load(&sched.staging_merge_complete);
+    TEST_ASSERT_EQUAL_INT(1, guard_fires_unfixed);
+}
+
 static void
 test_meet_host_selection_numeric_match(void)
 {
@@ -359,6 +391,7 @@ int main(int argc, char* argv[])
     TEST(test_meet_host_selection_always_true_passes);
     TEST(test_meet_host_selection_always_false_fails);
     TEST(test_gc_predicates_idle_invariant);
+    TEST(test_staging_merge_complete_gc_guard);
     TEST(test_meet_host_selection_numeric_match);
     TEST(test_meet_host_selection_string_match);