/* test_sched_worker_change.c — regression test for bug-1578 * * bug-1578: sched_worker_change() (sched.c) aliases w->host.resvals_to_sched * / nresvals_to_sched directly onto a local xlist_t's backing array * (resvalist.data), pushes it to the scheduler connection, then calls * xlist_fini(&resvalist) -- which frees that backing array -- WITHOUT first * resetting w->host.resvals_to_sched to NULL / nresvals_to_sched to 0. That * left a DANGLING pointer + a stale nonzero count on the worker_t struct for * the rest of that worker's lifetime; the next code to read * resvals_to_sched (production: persist_worker_hook_fun -> * capnp_encode_worker -> encode_Worker -> encode_NameValue_list -> * encode_NameValue, on worker disconnect) dereferenced freed memory -> * SIGSEGV. * * This bug had ONE PRIOR fix attempt (commit e5fde7b5) that silently never * took effect because it edited code inside the same dead "#if 0" (FIXME- * commented) block sched_worker_change() has always had (introduced * already-disabled by commit acfbec875) -- i.e. this exact bug * already regressed once before, silently, for lack of test coverage. The * real fix (2026-08-18) replaces that dead block with a direct * `w->host.nresvals_to_sched = 0; w->host.resvals_to_sched = NULL;` before * `xlist_fini(&resvalist);`, mirroring the sibling apiserver_worker_change() * (apiserver.c) that never had this bug. * * Fixture: sched_worker_change()/sched_worker_dead() are static to sched.c, * so this file drives them through the same public entry points production * code uses, exactly mirroring test_job_array_sched_push.c's established * pattern for this same standalone-binary strategy (see meson.build's * comment on sched_push_unit_test for why sched.c/dataex.c/egress.c must be * linked for REAL here rather than folded into controller_unit_test, and * why check_worker_() below is stubbed rather than linking all of opget.c): * on_scheduler_join() installs sched_worker_hook_fun into a real * cluster_t's hooksys (once per process -- hook_installed is a static * inside on_scheduler_join), and xce_run_hooks() fires * XCE_CONTROLLER_HOOK_WORKER / HOOK_DETAIL_WORKER_DEAD exactly as * handle_worker_leave() -> mark_worker_unavail() does in production. * * Test 1 asserts the exact condition the fix guarantees and the bug * violated: after the hook fires, w->host.resvals_to_sched == NULL and * w->host.nresvals_to_sched == 0 -- never a dangling pointer + stale count. * * Test 2 goes further and proves it under ASAN: immediately after the hook * fires, it calls the REAL production encoder encode_Worker() (controller. * capnp.c, the exact function bug-1578's crash backtrace bottoms out in -- * capnp_encode_worker/persist_worker_hook_fun's own callee) directly on the * SAME worker, exactly as persist_worker_hook_fun does on worker disconnect. * Post-fix this is a correct, harmless 0-entry encode. If the fix ever * regresses (nresvals_to_sched stale-nonzero, resvals_to_sched dangling), * encode_NameValue_list's loop reads resvals_to_sched[i] out of the freed * backing array -- an immediate ASAN heap-use-after-free, not just a failed * assertion. */ #include #include #include #include #include #include #include "cluster.h" #include "controller.capnp.h" #include "hook.h" #include "kvmgr.h" #include "netio.h" #include "request.h" #include "resobj.h" #include "xce/cluster_def.h" #include "xce/types.h" #include "xlist.h" #include "xstring.h" #include "zarray.h" extern int on_scheduler_join(cluster_t*); /* This file builds a STANDALONE test binary (meson.build's * sched_worker_change_unit_test) linking the real sched.c + its real * dataex.c/egress.c dependencies, mirroring sched_push_unit_test exactly * (see meson.build's comment on that target for why: several existing * controller_unit_test_src files already provide their OWN local fake * definitions of on_scheduler_join()/notify()/collect_confvars(), so linking * the real sched.c/dataex.c into that shared binary collides at link time). * opget.c's check_worker_() is sched.c's only remaining unresolved * reference (from xcesched_push_workers(), reached only through * on_scheduler_join()'s push-on-join branch, which this file's setup * deliberately never triggers at runtime -- c->sched.ep is NULL during that * call -- but the symbol must still resolve at link time). Stubbed locally * rather than linking all of opget.c, identical to test_job_array_sched_ * push.c's own stub. */ int check_worker_(xce_worker_t* w) { return (w != NULL) ? 1 : 0; } /* bug-1693: sched.c's handle_scheduler_leave() now calls * schedadmin_signal_disconnect() (defined for real in cluster.c, which this * standalone binary does not link -- see this file's own header comment * for why real sched.c/dataex.c/egress.c are linked here in isolation). * check_worker_() just above is a pure link-satisfying no-op stub since * this file's own tests never reach it at runtime; this one is instead a * counting SPY -- sched.c IS linked for real here, so handle_scheduler_ * leave() itself (unlike the OPCode_schedAdmin dispatch/wait machinery, * which lives in cluster.c and is NOT linked here) is exercisable for * real. test_scheduler_leave_signals_schedadmin_before_clearing_ep below * uses this spy to prove handle_scheduler_leave() actually calls this * function (not just that it compiles/links) -- the one piece of the * bug-1693 fix that test_sched_admin_dispatch.c's own regression test * (controller_unit_test, where cluster.c IS real but sched.c is stubbed by * test_config_repl.c) cannot reach, since the two real halves of this fix * (cluster.c's schedadmin_signal_disconnect() and sched.c's * handle_scheduler_leave()) are never both linked-for-real in the same * binary anywhere in this project's current test topology. */ static int g_schedadmin_signal_disconnect_calls; static cluster_t* g_schedadmin_spy_c; /* set by the test below; NULL elsewhere */ static endpoint_t* g_schedadmin_spy_ep_at_signal_time = (endpoint_t*)(uintptr_t)0x1; /* poison -- must be overwritten */ int schedadmin_signal_disconnect(void) { g_schedadmin_signal_disconnect_calls++; /* Captures c->sched.ep's value AT THE MOMENT this is called -- proves * handle_scheduler_leave() calls this BEFORE nulling c->sched.ep, not * after (see this fix's lock-ordering rationale in sched.c's doc * comment: calling it after would deadlock against the OPCode_ * schedAdmin dispatch case in production, which holds c->sched.lock for * its whole wait). */ if (g_schedadmin_spy_c != NULL) g_schedadmin_spy_ep_at_signal_time = g_schedadmin_spy_c->sched.ep; return 1; } /* ------------------------------------------------------------------ */ /* Fixture */ /* ------------------------------------------------------------------ */ static cluster_t g_swc_c; static endpoint_t g_swc_ep; static int g_swc_hook_wired; static var g_swc_resmgr; static void swc_val_killer(void* p) { free(p); } static void swc_drain_ep(endpoint_t* ep) { data_item_t* it; while ((it = endpoint_deq(ep)) != NULL) { io_packet_t* pkt = ITPTR_TO_PACKET(it); free_packet(pkt); free(pkt); } } /* Registers one resource ("mem", numeric) in a fresh resmgr and returns its * xce_resource_t* (idx assigned by makeResource/key_add, matching production * cluster_build_resources()'s own makeResource() call). */ static xce_resource_t* swc_setup(void) { kvmgr_params_t resmgr_params = { .key_type = KVMGR_KEY_STRING, .nslots = 64, .delta = 16, .datakiller = NULL }; xce_resource_t rdef; xce_resource_t* r; if (!g_swc_hook_wired) { memset(&g_swc_c, 0, sizeof(g_swc_c)); TEST_ASSERT_EQUAL_INT( 0, xce_hook_init(&g_swc_c.hooksys, XCE_CONTROLLER_HOOK_MAX)); /* sched.ep == NULL here so on_scheduler_join's push-on-join branch * is a no-op -- this call's only effect is installing * sched_worker_hook_fun (and siblings) into g_swc_c.hooksys, once, * process-wide (hook_installed is a static inside * on_scheduler_join). */ g_swc_c.sched.ep = NULL; TEST_ASSERT_EQUAL_INT(0, on_scheduler_join(&g_swc_c)); g_swc_hook_wired = 1; } g_swc_resmgr = makeInstance(&xkvmgr_type, &resmgr_params); TEST_ASSERT_NOT_NULL(g_swc_resmgr); memset(&rdef, 0, sizeof(rdef)); rdef.name = (char*)"mem"; rdef.valtype = XCE_VAL_NUMERIC; rdef.idx = -1; r = makeResource(g_swc_resmgr, &rdef); TEST_ASSERT_NOT_NULL(r); memset(&g_swc_ep, 0, sizeof(g_swc_ep)); g_swc_ep.fd = -1; /* c->sched.ep != NULL is sched_worker_change()'s guard for actually * pushing to the scheduler (situation == HOOK_AFTER_EVENT branch) -- * required to reach the collect_resval()/resvalist code path under * test at all. A zeroed, fd=-1 stack endpoint_t queues packets on * ep->outq without touching a real socket, exactly as test_job_array_ * sched_push.c's own fixture does for the JOB hook. */ g_swc_c.sched.ep = &g_swc_ep; g_swc_c.resmgr = g_swc_resmgr; g_swc_c.running = 1; g_swc_c.myhost = &g_swc_c.myhost_info; g_swc_c.myhost_info.name = (char*)"ctrl-swc"; g_swc_c.myhost_info.key = (char*)""; return r; } static void swc_teardown(void) { swc_drain_ep(&g_swc_ep); g_swc_c.sched.ep = NULL; g_swc_c.resmgr = NULL; if (g_swc_resmgr != NULL) { dereference(g_swc_resmgr); g_swc_resmgr = NULL; } } /* Builds a plain, stack-owned worker_t with one populated resval -- NOT * routed through makeWorker()/wobj.c's RTTI worker-object machinery * (unneeded: sched_worker_change() only ever reads w->host.name/resvals and * w->idx, none of which requires a real registered worker instance), * mirroring test_job_array_sched_push.c's identical "plain struct, not * through the real object constructor" rationale for its job_t fixtures. * The resval is written the same way update_val_res() (worker.c) writes a * real resupdate: a heap xce_val_t* stored via zarray_put() at the * resource's own idx. */ static void swc_make_worker(worker_t* w, const char* name, xce_resource_t* r, double fval) { xce_val_t* v; memset(w, 0, sizeof(*w)); w->host.name = (char*)name; w->idx = 0; zarray_init(&(w->host.resvals), 16, swc_val_killer); v = (xce_val_t*)calloc(1, sizeof(*v)); TEST_ASSERT_NOT_NULL(v); v->which = Value_u_fvalue; v->u.fvalue = fval; TEST_ASSERT_TRUE(zarray_put(&(w->host.resvals), r->idx, v) >= 0); } static void swc_free_worker(worker_t* w) { zarray_fini(&(w->host.resvals)); } static int swc_fire_worker_dead(worker_t* w) { return xce_run_hooks(&g_swc_c.hooksys, XCE_CONTROLLER_HOOK_WORKER, HOOK_AFTER_EVENT, HOOK_DETAIL_WORKER_DEAD, &g_swc_c, w); } /* Mirrors capnp_encode_worker() (persist.c) / tace_encode_worker() * (test_apply_committed_entry.c) exactly -- the real production helper that * wraps encode_Worker() for a malloc'd Cap'n Proto buffer. Not linking * persist.c itself (heavy, unrelated dependency chain for this fix); calling * encode_Worker() directly reaches the identical bug-1578 crash-site * function (controller.capnp.c) with zero loss of fidelity -- persist.c's * capnp_encode_worker() is a 20-line pass-through to it. */ static int swc_encode_worker(const xce_worker_t* w, uint8_t** out, size_t* out_len) { struct capn c; struct capn_segment* cs; struct Worker cw; Worker_ptr wp; capn_init_malloc(&c); cs = capn_root(&c).seg; encode_Worker(cs, &cw, (xce_worker_t*)w); wp = new_Worker(cs); write_Worker(&cw, wp); capn_setp(capn_root(&c), 0, wp.p); *out_len = capn_size(&c); *out = malloc(*out_len); if (*out == NULL) { capn_free(&c); return -ENOMEM; } capn_write_mem(&c, *out, *out_len, 0); capn_free(&c); return 0; } /* ------------------------------------------------------------------ */ /* Test 1: the exact condition the fix guarantees */ /* ------------------------------------------------------------------ */ static void test_worker_dead_leaves_resvals_to_sched_null_and_zero(void) { xce_resource_t* r; worker_t w; int rc; r = swc_setup(); swc_make_worker(&w, "swc-worker-1", r, 42.5); rc = swc_fire_worker_dead(&w); TEST_ASSERT_EQUAL_INT(0, rc); /* bug-1578: before the fix, sched_worker_change() aliased these two * fields directly onto a local xlist_t's backing array, then freed that * array (xlist_fini) WITHOUT resetting them -- leaving * resvals_to_sched a dangling pointer and nresvals_to_sched a stale * nonzero count. This is the exact condition the fix guarantees. */ TEST_ASSERT_NULL_MESSAGE( w.host.resvals_to_sched, "bug-1578 regression: w->host.resvals_to_sched must be NULL after " "sched_worker_change() completes -- a non-NULL value here is a " "dangling pointer into memory sched_worker_change() already freed " "via xlist_fini(&resvalist)"); TEST_ASSERT_EQUAL_INT_MESSAGE( 0, w.host.nresvals_to_sched, "bug-1578 regression: w->host.nresvals_to_sched must be 0 after " "sched_worker_change() completes -- a nonzero stale count paired " "with the dangling pointer above is exactly what let production " "code (persist_worker_hook_fun -> capnp_encode_worker) iterate " "into freed memory and SIGSEGV"); swc_free_worker(&w); swc_teardown(); printf( "test_worker_dead_leaves_resvals_to_sched_null_and_zero: PASS\n"); } /* ------------------------------------------------------------------ */ /* Test 2: ASAN-strength proof -- re-run the real crash-site encoder */ /* ------------------------------------------------------------------ */ static void test_worker_dead_then_encode_worker_is_asan_clean(void) { xce_resource_t* r; worker_t w; int rc; uint8_t* buf = NULL; size_t len = 0; r = swc_setup(); swc_make_worker(&w, "swc-worker-2", r, 7.0); rc = swc_fire_worker_dead(&w); TEST_ASSERT_EQUAL_INT(0, rc); /* Exactly what persist_worker_hook_fun() does on the SAME * HOOK_DETAIL_WORKER_DEAD event in production (capnp_encode_worker -> * encode_Worker -> encode_NameValue_list -> encode_NameValue). Post-fix, * w.host.nresvals_to_sched == 0, so encode_NameValue_list's loop never * executes -- a correct, harmless empty encode. If sched_worker_change() * ever regresses to the pre-fix behavior (dangling resvals_to_sched + * stale nonzero nresvals_to_sched), this call reads resvals_to_sched[i] * out of memory sched_worker_change() already freed -- an ASAN * heap-use-after-free abort, not merely a failed assertion. */ rc = swc_encode_worker(&w.host, &buf, &len); TEST_ASSERT_EQUAL_INT_MESSAGE( 0, rc, "encode_Worker() of the just-processed worker must succeed"); TEST_ASSERT_NOT_NULL(buf); TEST_ASSERT_TRUE(len > 0); free(buf); swc_free_worker(&w); swc_teardown(); printf( "test_worker_dead_then_encode_worker_is_asan_clean: PASS\n"); } /* ------------------------------------------------------------------ */ /* bug-1693: handle_scheduler_leave() signals schedadmin BEFORE clearing */ /* c->sched.ep (Decision Log fidelity fix) */ /* ------------------------------------------------------------------ */ /* Complements test_sched_admin_dispatch.c's own bug-1693 regression test * (controller_unit_test): that file proves schedadmin_signal_disconnect() * (cluster.c, real there) genuinely wakes a real pending OPCode_schedAdmin * wait immediately, but cannot exercise the real handle_scheduler_leave() * (sched.c is stubbed by test_config_repl.c in that binary). This binary * has the opposite shape: sched.c is real, cluster.c is not linked at all, * so schedadmin_signal_disconnect() above is a counting spy rather than the * real implementation. Together the two prove both halves of the fix for * real: the signal function's real effect on a real wait (there), and that * handle_scheduler_leave() really calls it, unconditionally and before * clearing c->sched.ep (here). */ static void test_scheduler_leave_signals_schedadmin_before_clearing_ep(void) { cluster_t* c = calloc(1, sizeof(cluster_t)); TEST_ASSERT_NOT_NULL(c); mylock_init(&(c->sched.lock)); endpoint_t sentinel; memset(&sentinel, 0, sizeof(sentinel)); c->sched.ep = &sentinel; g_schedadmin_signal_disconnect_calls = 0; g_schedadmin_spy_c = c; g_schedadmin_spy_ep_at_signal_time = (endpoint_t*)(uintptr_t)0x1; int rc = handle_scheduler_leave(c); TEST_ASSERT_EQUAL_INT(0, rc); TEST_ASSERT_EQUAL_INT_MESSAGE( 1, g_schedadmin_signal_disconnect_calls, "bug-1693 regression: handle_scheduler_leave() must call " "schedadmin_signal_disconnect() exactly once on every disconnect -- " "pre-fix, it never called it at all, so a pending OPCode_schedAdmin " "waiter could only ever be woken by the 5s deadline"); TEST_ASSERT_EQUAL_PTR_MESSAGE( &sentinel, g_schedadmin_spy_ep_at_signal_time, "handle_scheduler_leave() must call schedadmin_signal_disconnect() " "BEFORE nulling c->sched.ep -- calling it after would deadlock " "against the OPCode_schedAdmin dispatch case in production, which " "holds c->sched.lock for its entire wait duration (see the " "lock-ordering rationale in sched.c's handle_scheduler_leave() doc " "comment)"); TEST_ASSERT_NULL_MESSAGE(c->sched.ep, "handle_scheduler_leave() must still clear " "c->sched.ep as before -- this fix only adds " "the signal call, it does not change the " "pre-existing ep-clearing behavior"); g_schedadmin_spy_c = NULL; mylock_fini(&(c->sched.lock)); free(c); printf( "test_scheduler_leave_signals_schedadmin_before_clearing_ep: PASS\n"); } /* ------------------------------------------------------------------ */ /* Standalone Unity main (this binary is its own test executable -- see */ /* meson.build's sched_worker_change_unit_test) */ /* ------------------------------------------------------------------ */ void setUp(void) { return; } void tearDown(void) { return; } int main(void) { printf("\n>> bug-1578: sched_worker_change() dangling-pointer " "regression test\n\n"); /* makeResource()/makeString() (resobj.c/xstring.c) need the global * xstring intern table initialized before first use -- same requirement * test_worker_join.c documents at its own RUN_TEST call site. This is a * standalone binary (no other test file shares process init with it), * so it must be done exactly once here. */ xstring_table_init(139); UNITY_BEGIN(); RUN_TEST(test_worker_dead_leaves_resvals_to_sched_null_and_zero); RUN_TEST(test_worker_dead_then_encode_worker_is_asan_clean); RUN_TEST(test_scheduler_leave_signals_schedadmin_before_clearing_ep); return UNITY_END(); }