lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(worker): remove_worker frees every string add_worker copied (bug-1866)

Browse source 
add_worker strndup's launch_type and the four queue/job pre/post-exec
commands out of the startinfo, but remove_worker only freed exechome,
execcwd, execuser, job_cmd, pids and exdata, so every finished job leaked
up to five strings (LeakSanitizer at xcecd_virtual exit reported the
launch_type one).

New test in test_finished_job_retention.c compares ASAN's live-heap byte
count across an add/remove round trip (the worker suite runs with
detect_leaks=0): RED leaked exactly the 58 bytes of those five strings,
GREEN after. Worker suite 12/12.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-27 13:14:14 +00:00
parent 926104728d
commit e84a8d4332
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
2 changed files with 56 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/worker/src/xcecd_worker_utils.c
View file

@ -297,6 +297,12 @@ void remove_worker(xcecd_worker_t** worker_list, int worker_pid)
FREEUP(worker->job_cmd);
FREEUP(worker->job_info.pidinfo.pids);
FREEUP(worker->job_exdata);
/* bug-1866: every other string add_worker() copies */
FREEUP(worker->launch_type);
FREEUP(worker->q_pre_exec);
FREEUP(worker->q_post_exec);
FREEUP(worker->job_pre_exec);
FREEUP(worker->job_post_exec);
FREEUP(worker);
}
}

50
src/worker/testsuite/test_finished_job_retention.c
View file

@ -371,6 +371,55 @@ static void test_unhealthy_connection_suspends_expiry(void)
70005, finish_time_ms + window_ms + 100, &out));
}
/* ════════════════════════════════════════════════════════════════════════
* bug-1866: remove_worker() must free every string add_worker() copied
* out of the startinfo. This suite runs with detect_leaks=0, so compare
* ASAN's live-heap byte count across an add/remove round trip instead.
* ════════════════════════════════════════════════════════════════════════ */
#ifdef __SANITIZE_ADDRESS__
/* from <sanitizer/allocator_interface.h>, which the nix toolchain lacks */
size_t __sanitizer_get_current_allocated_bytes(void);
static void add_remove_full_startinfo_worker(int pid, uint64_t jobid)
{
xcecd_worker_t* list = NULL;
job_startinfo_t startinfo = {
.exechome = (char*)"/home/u",
.execcwd = (char*)"/home/u/run",
.execuser = (char*)"u",
.cmdline = (char*)"sleep 1",
.exdata = (char*)"exdata",
.launch_type = (char*)"openmpi",
.q_pre_exec = (char*)"/bin/q_pre",
.q_post_exec = (char*)"/bin/q_post",
.job_pre_exec = (char*)"/bin/job_pre",
.job_post_exec = (char*)"/bin/job_post",
};
add_worker(&list, pid, WORKER_RUNNING, jobid, 1, &startinfo);
TEST_ASSERT_NOT_NULL(list);
remove_worker(&list, pid);
TEST_ASSERT_NULL(list);
}
#endif
static void test_remove_worker_frees_every_startinfo_copy(void)
{
#ifdef __SANITIZE_ADDRESS__
/* first round trip settles any lazy allocation (logging etc.) */
add_remove_full_startinfo_worker(4101, 91001);
size_t before = __sanitizer_get_current_allocated_bytes();
add_remove_full_startinfo_worker(4102, 91002);
size_t after = __sanitizer_get_current_allocated_bytes();
TEST_ASSERT_EQUAL_UINT64((uint64_t)before, (uint64_t)after);
#else
TEST_IGNORE_MESSAGE("needs AddressSanitizer's allocator statistics");
#endif
}
/* ════════════════════════════════════════════════════════════════════════
* main
* ════════════════════════════════════════════════════════════════════════ */
@ -391,6 +440,7 @@ int main(void)
RUN_TEST(test_lookup_after_expiry_prunes_entry_permanently);
RUN_TEST(test_reconnect_restarts_window_for_outcome_finished_before_outage);
RUN_TEST(test_unhealthy_connection_suspends_expiry);
RUN_TEST(test_remove_worker_frees_every_startinfo_copy);
return UNITY_END();
}