diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index f8ae710b..17d26d5f 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -1816,7 +1816,8 @@ int cmd_jobsub(int argc, char* argv[]) default: printf( "jobsub -u -q -R -c \n"); - return -EINVAL; + rc = -EINVAL; + goto cleanup; } } @@ -1827,17 +1828,20 @@ int cmd_jobsub(int argc, char* argv[]) if (NULLSTR(submit.user)) { printf("you need to specify job user with -u\n"); - return -EINVAL; + rc = -EINVAL; + goto cleanup; } if (NULLSTR(submit.queue)) { printf("you need to specify job queue with -q\n"); - return -EINVAL; + rc = -EINVAL; + goto cleanup; } if (NULLSTR(submit.start_info->cmdline)) { printf("you need to specify job command with -c\n"); - return -EINVAL; + rc = -EINVAL; + goto cleanup; } if (NULLSTR(submit.resreq)) { @@ -1866,6 +1870,7 @@ int cmd_jobsub(int argc, char* argv[]) jobsubmit.array_size = arsize; rc = xcecli_jobsubmit(active_ctx->ctx, &jobsubmit, &jobid); +cleanup: FREEUP(submit.user); FREEUP(submit.queue); FREEUP(submit.resreq); diff --git a/src/include/client.h b/src/include/client.h index e7eb065a..0cc4a0ff 100644 --- a/src/include/client.h +++ b/src/include/client.h @@ -114,9 +114,12 @@ extern int xcecli_jobinfo(xcecli_ctx_t, uint64_t, xce_job_t**); #define xcecli_free_queues(nqueues, queues) xce_free_queues(nqueues, queues) #define xcecli_free_groups(ngroups, groups) xce_free_groups(ngroups, groups) #define xcecli_free_clusterconf(conf) xce_free_clusterconf(conf) -/* ctrlstatus reply is freed by the generated codec free (free_CtrlStatus, - * declared in the generated controller.capnp.h). */ -#define xce_free_ctrlstatus(s) free_CtrlStatus(s) +/* ctrlstatus reply is freed by the generated codec free. free_CtrlStatus() + * only frees the fields INSIDE the struct; the xce_ctrlstatus_t itself is + * calloc'd by decode_CtrlStatus_ptr() and must be freed via free_CtrlStatus_ptr() + * (which also nulls the caller's pointer) or every call leaks sizeof(xce_ctrlstatus_t) + * (bug-1401). */ +#define xce_free_ctrlstatus(s) free_CtrlStatus_ptr(&(s)) #define xcecli_free_ctrlstatus(s) xce_free_ctrlstatus(s) #endif diff --git a/src/lib/testsuite/test_ctrlstatus.c b/src/lib/testsuite/test_ctrlstatus.c index 60c5943c..c8c13903 100644 --- a/src/lib/testsuite/test_ctrlstatus.c +++ b/src/lib/testsuite/test_ctrlstatus.c @@ -192,6 +192,63 @@ static void test_ctrlstatus_roundtrip_unknown_primary(void) printf("test_ctrlstatus_roundtrip_unknown_primary: PASS\n"); } +/* bug-1401: xce_free_ctrlstatus() must free the heap-allocated xce_ctrlstatus_t + * itself, not just the fields inside it. decode_CtrlStatus_ptr() (the real + * client reply path, via xce_decode -> ... -> xce_ctrlstatus) calloc's the + * struct; free_CtrlStatus() alone only frees the inner strings/peers list and + * leaks the struct. This mirrors that allocation path (decode_CtrlStatus_ptr, + * not the stack-struct decode_CtrlStatus used by the roundtrip tests above) + * and relies on ASan leak detection to catch a regression. */ +static void test_xce_free_ctrlstatus_frees_the_struct_itself(void) +{ + xce_ctrlpeer_t p0 = { .name = "ctrl-a", .host = "10.0.0.1", .port = 9991 }; + xce_ctrlpeer_t* peerlist[1] = { &p0 }; + xce_ctrlstatus_t in; + memset(&in, 0, sizeof(in)); + in.clusterName = "lab-cluster"; + in.primaryName = "ctrl-a"; + in.responderName = "ctrl-a"; + in.configChecksum = "deadbeef"; + in.peers.count = 1; + in.peers.list = peerlist; + in.peerCount = 1; + + struct capn c; + struct capn_segment* cs; + struct CtrlStatus cap_in; + CtrlStatus_ptr sp; + + capn_init_malloc(&c); + cs = capn_root(&c).seg; + memset(&cap_in, 0, sizeof(cap_in)); + encode_CtrlStatus(cs, &cap_in, &in); + sp = new_CtrlStatus(cs); + write_CtrlStatus(&cap_in, sp); + capn_setp(capn_root(&c), 0, sp.p); + + size_t sz = capn_size(&c); + uint8_t* buf = (uint8_t*)malloc(sz); + TEST_ASSERT_NOT_NULL(buf); + capn_write_mem(&c, buf, sz, 0); + capn_free(&c); + + struct capn dc; + CtrlStatus_ptr dp; + capn_init_mem(&dc, buf, sz, 0); + dp.p = capn_getp(capn_root(&dc), 0, 1); + + xce_ctrlstatus_t* out = NULL; + decode_CtrlStatus_ptr(&out, dp); + TEST_ASSERT_NOT_NULL(out); + + xce_free_ctrlstatus(out); + + capn_free(&dc); + free(buf); + + printf("test_xce_free_ctrlstatus_frees_the_struct_itself: PASS\n"); +} + /* ── no reachable controller ⇒ typed failure (Req 7.3) ──────────────────── */ static void test_xcecli_ctrlstatus_null_ctx_typed_failure(void) @@ -396,6 +453,7 @@ void test_ctrlstatus(void) RUN_TEST(test_handle_reply_ctrlstatus_does_not_free_foreign_slot); RUN_TEST(test_ctrlstatus_roundtrip_all_fields); RUN_TEST(test_ctrlstatus_roundtrip_unknown_primary); + RUN_TEST(test_xce_free_ctrlstatus_frees_the_struct_itself); RUN_TEST(test_xcecli_ctrlstatus_null_ctx_typed_failure); RUN_TEST(test_xcecli_ctrlstatus_primary_null_ctx_typed_failure); RUN_TEST(test_xcecli_ctrlstatus_primary_null_out_typed_failure); diff --git a/src/tools/test-commit-durability.sh b/src/tools/test-commit-durability.sh index fa35973f..faf2c78d 100755 --- a/src/tools/test-commit-durability.sh +++ b/src/tools/test-commit-durability.sh @@ -392,7 +392,15 @@ assert_job_count() { pass "$label: cluster lists exactly $expected jobs" return 0 else - fail "$label: job count != $expected after recovery (see expect-jobs output above)" + # bug-1396: a bare count mismatch isn't enough to diagnose an extra-job + # anomaly — dump the live joblist and the harness's own ACKED_FILE + # ledger side by side so the diverging job ID is visible in the log + # instead of only the count. + yellow " $label: job count mismatch — dumping live joblist vs ACKED_FILE for diagnosis" + xce_test_script "$(printf 'set masters %s\nclient\njoblist\n' "$MASTERS")" + echo " --- ACKED_FILE (tracked committed jobs, deduplicated) ---" + sort -u "$ACKED_FILE" | sed 's/^/ /' + fail "$label: job count != $expected after recovery (see joblist/ACKED_FILE dump above)" return 1 fi }