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

fix(scheduler): never elect a job without a schedobj (bug-1897)

Browse source 
A job whose snapshot resreq fails to parse (any replay captured before
the bug-1888 fix) is kept by xce_sched_core_load_snapshot() with no
schedobj, but was still made a picker candidate -- by the loader and, in
simulation mode, by sim_bootstrap_picker()'s picker rebuild. The
allocator thread then dereferenced the NULL schedobj in
limit_job_election() and xce_scheduler --sim crashed.

Both push sites now skip such a job (it stays in the core so a restored
allocation is still charged), and sched_allocator() skips any picked job
without a schedobj as a backstop.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-01 03:45:51 +00:00
parent 3d3f37e0fd
commit 0fe27d4be3
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
7 changed files with 239 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

10
src/scheduler/src/sched_election.c
View file

@ -230,6 +230,16 @@ void* sched_allocator(void* arg)
continue;
}
/* bug-1897: election and allocation both work on job->schedobj; a
* job without one (its resreq failed to parse) cannot be scheduled.
* Skip it like a job that cannot be found. */
if (job->schedobj == NULL) {
fmt_jobid(jobidString, sizeof(jobidString), jobid);
ERROR("%s job %s has no scheduling object, skipped", __func__,
jobidString);
continue;
}
/* scheduler-sim-live-allocation spec, task 1.2: plugin_job_election()
* is core-typed (mirrors plugin_begin_session(core) in this same
* codebase's driver files) -- pass core directly. */

5
src/scheduler/src/sim_picker.c
View file

@ -114,6 +114,11 @@ void sim_bootstrap_picker(xce_sched_sim_driver_t* sim)
if ((rc < 0) || (rc == RTTI_ITER_NO_DATA)) {
break;
}
/* bug-1897: same skip as the load itself -- a job
* without a schedobj is never a picker candidate. */
if (job->schedobj == NULL) {
continue;
}
plugin->picker_iface->on_job_push(plugin, job);
}
iter_destroy(&it);

19
src/scheduler/src/snapshot_load.c
View file

@ -366,10 +366,9 @@ int xce_sched_core_load_snapshot(xce_sched_core_t* core,
/* scheduler-snapshot-job-allocation task 1.2 (Req 2 AC 1-4):
* construct job->schedobj for every restored job whose resreq
* parsed successfully -- guarded on job->resreq != NULL so the
* pre-existing, unrelated resreq-parse-failure path above
* (which already leaves job->resreq NULL) is tolerated, not
* fixed, here (design.md "Load-Side Job Reconstruction", Known
* limitation). */
* resreq-parse-failure path above (which leaves job->resreq
* NULL) is tolerated here; such a job is kept out of the
* picker below (bug-1897). */
if (job->resreq != NULL) {
job->schedobj
= (xce_sched_obj_t*)calloc(1, sizeof(*(job->schedobj)));
@ -403,7 +402,17 @@ int xce_sched_core_load_snapshot(xce_sched_core_t* core,
sched_add_job(core, job);
if (picker != NULL) {
/* bug-1897: a job without a schedobj (resreq parse failure
* above, or its calloc failing) stays in the core so its
* restored allocation is still charged and later events still
* find it, but must never become a picker candidate --
* sched_allocator() runs plugin_job_election() and
* sched_make_allocation() on job->schedobj. */
if (job->schedobj == NULL) {
ERROR("%s jobid %llu has no scheduling object; it will "
"not be scheduled",
__func__, (unsigned long long)wj->jobid);
} else if (picker != NULL) {
picker->picker_iface->on_job_push(picker, job);
}
}

21
src/scheduler/testsuite/meson.build
View file

@ -2216,6 +2216,27 @@ test('scheduler election sim fixture test',
sched_election_sim_fixture_test, suite: 'scheduler',
env: ['ASAN_OPTIONS=detect_leaks=0'])
# bug-1897: sched_allocator() must skip a picked job whose schedobj is NULL
# (resreq failed to parse on snapshot load) instead of dereferencing it.
# Same real-scheduler_srcs link technique as the fixture test above.
sched_election_null_schedobj_test_srcs = ['test_sched_election_null_schedobj.c']
foreach f : scheduler_srcs
if f != 'src/main.c'
sched_election_null_schedobj_test_srcs += ['../' + f]
endif
endforeach
sched_election_null_schedobj_test = executable(
'sched_election_null_schedobj_test',
sched_election_null_schedobj_test_srcs,
include_directories:[incdir],
dependencies: [scheduler_test_deps],
install: false)
test('scheduler election null schedobj test',
sched_election_null_schedobj_test, suite: 'scheduler',
env: ['ASAN_OPTIONS=detect_leaks=0'])
# scheduler-sim-live-allocation spec, task 4.1 (Req 1.1, 1.2, 3.3, 4.2, 4.3,
# 5.1, boundary: Unified Picker/Allocator Threads, Sim Driver): proves
# sched_sim_driver_run() (sim_loop.c) itself -- not just the relocated

115
src/scheduler/testsuite/test_sched_election_null_schedobj.c Normal file
View file

@ -0,0 +1,115 @@
/*
* test_sched_election_null_schedobj.c - bug-1897.
*
* sched_allocator() (sched_election.c) runs plugin_job_election() and
* sched_make_allocation() on job->schedobj. A job whose resreq could not be
* parsed has no schedobj (snapshot_load.c keeps such a job so a running
* job's allocation stays charged), and before the fix the allocator thread
* dereferenced the NULL schedobj (limit_job_election(), limit.c:336, in a
* replay; sched_make_allocation()'s obj->tag here, with no limit plugin).
*
* The allocator must skip such a job -- no decision for it -- and carry on
* with the session: the only decisions-cbuf entry is the end-of-session
* NULL marker.
*
* Fixture as in test_sched_election_sim_fixture.c (`wrapper` is a real
* xce_scheduler_t used only for plugin registration; the threads get the
* bare core). One RUN_TEST() only: the threads are never joined and
* reference stack-local state.
*/
#include <errno.h>
#include <pthread.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <unity/unity.h>
#include "decision.h"
#include "plugin.h"
#include "sched/types.h"
#include "scheduler.h"
#include "xstring.h"
#define TEST_JOBID ((uint64_t)189701ULL)
#define QUEUE_NAME "normal"
void setUp(void) { return; }
void tearDown(void) { return; }
static void test_sched_allocator_skips_job_without_schedobj(void)
{
xce_scheduler_t wrapper;
xce_sched_core_t* core;
xce_sched_plugin_t* fcfs;
xce_sched_queue_t* queue;
xce_sched_job_t* job;
pthread_t picker_tid;
pthread_t allocator_tid;
xce_decision_t* decision = (xce_decision_t*)0x1;
int rc;
memset(&wrapper, 0, sizeof(wrapper));
rc = xce_sched_core_init(&(wrapper.core));
TEST_ASSERT_EQUAL_INT(0, rc);
core = &(wrapper.core);
fcfs = builtin_fcfs_new_plugin();
TEST_ASSERT_NOT_NULL(fcfs);
rc = plugin_register(&wrapper, fcfs);
TEST_ASSERT_TRUE(rc >= 0);
core->job_pick_plugin = (char*)"builtin_fcfs";
rc = register_seqjob_allocator(&wrapper);
TEST_ASSERT_TRUE(rc >= 0);
rc = register_parjob_allocator(&wrapper);
TEST_ASSERT_TRUE(rc >= 0);
queue = (xce_sched_queue_t*)calloc(1, sizeof(*queue));
TEST_ASSERT_NOT_NULL(queue);
queue->name = makeString(QUEUE_NAME);
rc = sched_add_queue(core, queue);
TEST_ASSERT_TRUE(rc >= 0);
fcfs->picker_iface->on_queue_update(fcfs, queue);
/* The shape snapshot_load.c leaves for a job whose resreq failed to
* parse: resreq and schedobj both NULL. Pushed to the picker directly,
* so this test covers the allocator guard on its own, independent of
* the loader no longer pushing such a job. */
job = (xce_sched_job_t*)calloc(1, sizeof(*job));
TEST_ASSERT_NOT_NULL(job);
job->jobid = TEST_JOBID;
job->state = JOB_STAT_PEND;
job->queue = queue;
job->user = sched_get_user(core, "alice");
rc = sched_add_job(core, job);
TEST_ASSERT_TRUE(rc >= 0);
fcfs->picker_iface->on_job_push(fcfs, job);
rc = pthread_create(&picker_tid, NULL, sched_job_picker, core);
TEST_ASSERT_EQUAL_INT(0, rc);
rc = pthread_create(&allocator_tid, NULL, sched_allocator, core);
TEST_ASSERT_EQUAL_INT(0, rc);
rc = xce_core_go(core);
TEST_ASSERT_EQUAL_INT(0, rc);
cbuf_get(&(core->decisions), &decision);
TEST_ASSERT_NULL_MESSAGE(decision,
"a job without a schedobj must be skipped: the "
"only entry is the end-of-session NULL marker");
rc = xce_core_end_session(core);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_UINT(JOB_STAT_PEND, job->state);
TEST_ASSERT_NULL(job->allocs);
}
int main(void)
{
UNITY_BEGIN();
RUN_TEST(test_sched_allocator_skips_job_without_schedobj);
return UNITY_END();
}

68
src/scheduler/testsuite/test_sim_event_loop.c
View file

@ -899,11 +899,13 @@ test_run_deactivates_tlog_virtual_clock_logging_on_completion(void)
* already white-box #included above) BEFORE calling sched_record_start() --
* so xce_sched_core_save_snapshot() (snapshot.c) captures both into the
* INITIAL snapshot itself, not as a later ReplayEvent. */
static FILE* write_header_and_snapshot_with_pending_job(char* out_path,
size_t out_path_len,
const char* suffix,
const char* queue_name,
uint64_t jobid)
static FILE*
write_header_and_snapshot_with_pending_job_resreq(char* out_path,
size_t out_path_len,
const char* suffix,
const char* queue_name,
uint64_t jobid,
const char* saved_resreq)
{
char record_dir[PATH_MAX];
xce_scheduler_t sched;
@ -945,6 +947,13 @@ static FILE* write_header_and_snapshot_with_pending_job(char* out_path,
TEST_ASSERT_NOT_NULL(job);
TEST_ASSERT_GREATER_OR_EQUAL(0, sched_add_job(&(sched.core), job));
/* bug-1897: the snapshot saves resreq_str verbatim, so overwriting it
* here yields a snapshot whose resreq the loader cannot parse. */
if (saved_resreq != NULL) {
FREEUP(job->resreq_str);
job->resreq_str = strdup(saved_resreq);
}
rc = sched_record_start(&sched, "fixture.replay");
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_TRUE(strlen(sched.record_file_path) < out_path_len);
@ -953,6 +962,16 @@ static FILE* write_header_and_snapshot_with_pending_job(char* out_path,
return sched.record_file;
}
static FILE* write_header_and_snapshot_with_pending_job(char* out_path,
size_t out_path_len,
const char* suffix,
const char* queue_name,
uint64_t jobid)
{
return write_header_and_snapshot_with_pending_job_resreq(
out_path, out_path_len, suffix, queue_name, jobid, NULL);
}
/* Same dup()/dup2() STDOUT_FILENO capture technique test_sim_verify.c's own
* run_sim_and_capture_stdout() establishes (this file's white-box TU does
* not #include that file, so a plain copy is needed here rather than a
@ -1065,6 +1084,44 @@ test_job_present_only_in_initial_snapshot_still_reaches_the_picker(void)
"picker: PASS\n");
}
/*
* bug-1897: a replay whose INITIAL snapshot holds a pending job with a
* resreq the loader cannot parse (e.g. bug-1888's old debug-dump text).
* The loader keeps the job without a schedobj; sim_bootstrap_picker()'s
* picker rebuild used to push it anyway, and the allocator thread then
* dereferenced the NULL schedobj (SEGV in limit_job_election()). The run
* must complete, with no decision for that job.
*/
static void test_snapshot_job_with_unparsable_resreq_is_not_scheduled(void)
{
char path[PATH_MAX];
char marker[80];
char jobid_buf[64];
FILE* f;
int rc;
char* captured;
f = write_header_and_snapshot_with_pending_job_resreq(
path, sizeof(path), "_badresreq", "normal", 4343ULL, "{");
TEST_ASSERT_NOT_NULL(f);
append_replay_session(f, 1000ULL);
append_end_of_stream_marker(f);
fclose(f);
captured = run_sim_and_capture_stdout(path, &rc);
fmt_jobid(jobid_buf, sizeof(jobid_buf), 4343ULL);
snprintf(marker, sizeof(marker), "jobid=%s", jobid_buf);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_NULL_MESSAGE(strstr(captured, marker),
"a job whose snapshot resreq cannot be parsed "
"must get no decision (bug-1897)");
free(captured);
}
int main(void)
{
UNITY_BEGIN();
@ -1076,6 +1133,7 @@ int main(void)
RUN_TEST(test_run_deactivates_tlog_virtual_clock_logging_on_completion);
RUN_TEST(
test_job_present_only_in_initial_snapshot_still_reaches_the_picker);
RUN_TEST(test_snapshot_job_with_unparsable_resreq_is_not_scheduled);
return UNITY_END();
}

16
src/scheduler/testsuite/test_snapshot_save_load.c
View file

@ -1313,11 +1313,11 @@ test_snapshot_load_computes_resreq_fingerprint_for_restored_primary(void)
0, restored->schedobj->u.primary->resreq_fp.select_node_id);
}
/* Regression-safety test (NOT a new feature): a job whose resreq fails to
* parse on load is a pre-existing, out-of-boundary gap (design.md "Known
* limitation") that task 1.2 must NOT try to fix -- it must only make sure
* schedobj construction is skipped rather than crashing on a NULL resreq.
* Proves job->schedobj stays NULL, identical to today's behavior. */
/* Regression-safety test: a job whose resreq fails to parse on load keeps
* job->resreq and job->schedobj NULL (schedobj construction is skipped
* rather than crashing on a NULL resreq, task 1.2), stays in the core, and
* -- bug-1897 -- is never pushed to the picker, so the allocator thread
* never elects a job with no schedobj. */
static void
test_snapshot_load_leaves_schedobj_null_when_resreq_fails_to_parse(void)
{
@ -1364,6 +1364,12 @@ test_snapshot_load_leaves_schedobj_null_when_resreq_fails_to_parse(void)
/* THIS feature's guard: schedobj construction is skipped, not crashed
* into, when resreq is NULL. */
TEST_ASSERT_NULL(restored->schedobj);
/* bug-1897: a job with no schedobj must never become a picker
* candidate -- the allocator thread runs plugin_job_election() on
* job->schedobj (limit_job_election() dereferenced it as NULL). */
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, g_stub_state_a.ncandidates,
"a job whose resreq failed to parse must not be pushed to the picker");
}
/* ---------------------------------------------------------------------- */