From 8ac637ad366ac4d8cd9740694902858ca34396f7 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 2 Oct 2026 02:26:38 +0000 Subject: [PATCH] fix(worker): xcecd_virtual ends a killed running job (bug-1905) The virtual worker's job-end decision lived inside the branches of the systemd exit-code check, and only the exit-code-0 branch looked at killed_signal. Without a system bus that call returns -1, so a kill was recorded but ignored until the job's sleep time elapsed. A virtual job has no systemd unit: decide its end from the kill and the timer alone, before asking systemd. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/worker/src/xcecd_dbus_manager.c | 40 +++++++-------- .../testsuite/test_dbus_workers_update.c | 50 +++++++++++++++++++ 2 files changed, 68 insertions(+), 22 deletions(-) diff --git a/src/worker/src/xcecd_dbus_manager.c b/src/worker/src/xcecd_dbus_manager.c index 259902e1..97cd99e2 100644 --- a/src/worker/src/xcecd_dbus_manager.c +++ b/src/worker/src/xcecd_dbus_manager.c @@ -249,22 +249,27 @@ int dbus_workers_update(xcecd_worker_t** worker_list, xcecd_job_t* job, continue; } +#ifdef VIRTUAL_CLUSTER + /* bug-1905: a virtual job has no systemd unit, so its end never + * depends on systemd's answer (-1 without a system bus, which + * used to skip the kill check): a kill ends it as EXITED with + * 128 + signal, otherwise it is DONE once its sleep time is up. */ + if (worker->killed_signal != 0) { + job->exit_code = 128 + worker->killed_signal; + ret = update_worker_status(*worker_list, worker->worker_pid, + WORKER_EXITED, job, db); + } else if (check_worker_sleep_time_elapsed(worker)) { + job->exit_code = 0; + ret = update_worker_status(*worker_list, worker->worker_pid, + WORKER_DONE, job, db); + } + worker = next; + continue; +#endif + /* check if worker exit or done */ exit_code = get_systemd_job_exit_code(unit_name); if (exit_code == 0) { -#ifdef VIRTUAL_CLUSTER - if (check_worker_sleep_time_elapsed(worker)) { - job->exit_code = 0; - ret = update_worker_status(*worker_list, worker->worker_pid, - WORKER_DONE, job, db); - } else if (worker->killed_signal != 0) { - job->exit_code = 128 + worker->killed_signal; - ret = update_worker_status(*worker_list, worker->worker_pid, - WORKER_EXITED, job, db); - } - worker = next; - continue; -#endif XCECD_LOG(XCE_LOG_DEBUG, "Worker %d is done", worker->worker_pid); @@ -284,15 +289,6 @@ int dbus_workers_update(xcecd_worker_t** worker_list, xcecd_job_t* job, WORKER_EXITED, job, db); } else { -#ifdef VIRTUAL_CLUSTER - if (check_worker_sleep_time_elapsed(worker)) { - job->exit_code = 0; - ret = update_worker_status(*worker_list, worker->worker_pid, - WORKER_DONE, job, db); - } - worker = next; - continue; -#endif XCECD_LOG(XCE_LOG_WARN, "Failed to get worker exit code"); job->exit_code = -1; diff --git a/src/worker/testsuite/test_dbus_workers_update.c b/src/worker/testsuite/test_dbus_workers_update.c index ca19ff83..8a3ad609 100644 --- a/src/worker/testsuite/test_dbus_workers_update.c +++ b/src/worker/testsuite/test_dbus_workers_update.c @@ -330,6 +330,54 @@ static void test_done_head_worker_does_not_block_rest_of_list(void) free_worker_list(list); } +/* ════════════════════════════════════════════════════════════════════════ + * bug-1905 — a killed running job ends as EXITED (128 + signal) on the next + * pass, even when systemd is unreachable (get_systemd_job_exit_code() -1: + * no system bus), instead of running until its sleep time elapses. + * ════════════════════════════════════════════════════════════════════════ */ + +static void test_killed_running_job_exits_without_systemd(void) +{ + xcecd_worker_t* list = NULL; + xcecd_worker_t* w = make_starting_worker(3000, 300); + + w->job_status = JOB_RUNNING; + w->sleep_time = 600; /* far from elapsed */ + w->killed_signal = 15; /* SIGTERM recorded by the job-control handler */ + link_worker(&list, w); + + xcecd_job_t job = { 0 }; + int ret = dbus_workers_update(&list, &job, NULL); + + TEST_ASSERT_EQUAL_INT(XCECD_OK, ret); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_uws_ncalls, + "the killed job must be reported once"); + TEST_ASSERT_EQUAL_INT(3000, g_uws_pid[0]); + TEST_ASSERT_EQUAL_INT(WORKER_EXITED, g_uws_worker_status[0]); + TEST_ASSERT_EQUAL_INT(128 + 15, job.exit_code); + + free_worker_list(list); +} + +/* A running job that was not killed keeps running until its time is up. */ +static void test_unkilled_running_job_keeps_running(void) +{ + xcecd_worker_t* list = NULL; + xcecd_worker_t* w = make_starting_worker(3001, 301); + + w->job_status = JOB_RUNNING; + w->sleep_time = 600; + link_worker(&list, w); + + xcecd_job_t job = { 0 }; + int ret = dbus_workers_update(&list, &job, NULL); + + TEST_ASSERT_EQUAL_INT(XCECD_OK, ret); + TEST_ASSERT_EQUAL_INT(0, g_uws_ncalls); + + free_worker_list(list); +} + /* ════════════════════════════════════════════════════════════════════════ * main * ════════════════════════════════════════════════════════════════════════ */ @@ -342,6 +390,8 @@ int main(void) RUN_TEST(test_all_array_elements_get_started_in_one_pass); RUN_TEST(test_done_head_worker_does_not_block_rest_of_list); + RUN_TEST(test_killed_running_job_exits_without_systemd); + RUN_TEST(test_unkilled_running_job_keeps_running); return UNITY_END(); }