fix(scheduler): a batch replay exits 0 under LeakSanitizer (bug-1884, part 6)

- kill_sched_resval() (every resource-value array's item killer) frees
  the value, not only a string's buffer: each replaced or torn-down value
  leaked its struct, including every WORKERS_PUSH value update.
- put_resvals() shares the value it creates with later entries for the
  same index in one call; it created a second one that replaced (and now
  freed) the first while still writing through it.
- JOB_UPDATE and the duplicate-JOBS_PUSH path kill_resreq() the old parsed
  resreq, and free the rejected job's schedobj (xce_core_free_schedobj(),
  exported from core.c), instead of freeing only the containers.
- The previous attempt's (or ALTER alternative's) pend reason is freed when
  a new one replaces it.
- kill_resreq_ctx() frees the parser context's error message, and the
  primary-resreq PEG action frees an earlier primary's message first.
- write_snapshot_entry() frees the snapshot it builds (config and resource
  objects detached, they are the core's); the save side strdup()s its
  strings so the generated free_SnapshotData() applies.

Test: test_cli_batch_replay_exits_clean_under_leak_detection
(test_sim_discrete_clock.c) runs the real binary on a two-session fixture
with detect_leaks=1 and requires exit 0: RED before (exit 1), GREEN now.
All nine HA-soak recordings replay with exit 0 and no LeakSanitizer report.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/lib/resreq.c

@@ -505,6 +505,9 @@ void kill_resreq_ctx(resreq_context_t* ctx)
     if (ctx == NULL) {
         return;
     }
+
+    /* bug-1884: a primary's parse error lands here (xce_resreq.peg) */
+    FREEUP(ctx->errmsg);
 }
 
 int parse_resreq(xce_resreq_t* resreq, char* data, var resmgr, char** errmsg)

src/lib/xce_resreq.peg

@@ -106,6 +106,8 @@ primary_resreq <- <  (! ('{' / '}' ) .)* > {
     xce_primary_resreq_t *primary = (xce_primary_resreq_t *)calloc(1, sizeof(*primary));
     if (primary != NULL) {
       auxil->primary = primary;
+      free(auxil->errmsg); /* bug-1884: an earlier primary's message */
+      auxil->errmsg = NULL;
       parse_resreq_primary(primary, (char *)$0, auxil->resmgr, &(auxil->errmsg));
     }
   }

src/scheduler/include/core.h

@@ -450,6 +450,12 @@ extern void xce_core_build_schedobj(xce_sched_obj_t*, const char* user,
                                     const char* queue, const char* project,
                                     xce_resreq_t*);
 
+/* xce_core_free_schedobj - bug-1884: frees a schedobj built by
+ * xce_core_build_schedobj() (names, limit lists, per-primary wrappers and
+ * their pend reasons, last allocation and reason). Its resreqs belong to
+ * the job and are not freed. NULL-safe. */
+extern void xce_core_free_schedobj(xce_sched_obj_t* obj);
+
 /*
  * sched_check_host_satisfy()/sched_check_shared_satisfy() - relocated here
  * from scheduler.h (scheduler-driver-equivalence fix, bug-1836), following

src/scheduler/src/core.c

@@ -209,7 +209,7 @@ int xce_sched_core_init(xce_sched_core_t* core)
  * used only by xce_sched_core_fini() after the table (and its shallow
  * datakiller) is gone -- see core_destroy_and_free_(). Here rather than in
  * schedobj.c so the core's teardown links on its own. */
-static void core_schedobj_free_(xce_sched_obj_t* obj)
+void xce_core_free_schedobj(xce_sched_obj_t* obj)
 {
     int i;
 
@@ -268,7 +268,7 @@ static void core_schedobj_free_(xce_sched_obj_t* obj)
 static void core_job_free_(xce_sched_job_t* job)
 {
     free_JobRusages_ptr(&(job->rusages));
-    core_schedobj_free_(job->schedobj);
+    xce_core_free_schedobj(job->schedobj);
     if (job->resreq != NULL) {
         kill_resreq(job->resreq);
         free(job->resreq);

src/scheduler/src/core_events.c

@@ -7,6 +7,7 @@
 #include "core.h"
 #include "misc.h"
 #include "resobj.h"
+#include "resreq.h"
 #include "sched/types.h"
 #include "utils.h"
 #include "xcealloc.h"
@@ -334,6 +335,7 @@ static xce_sched_job_t* core_build_job(xce_sched_core_t* core, xce_job_t* j)
 
     job->schedobj = (xce_sched_obj_t*)calloc(1, sizeof(*(job->schedobj)));
     if (job->schedobj == NULL) {
+        kill_resreq(job->resreq);
         FREEUP(job->resreq);
         FREEUP(job);
         return NULL;
@@ -1047,8 +1049,11 @@ int xce_core_push_events(xce_sched_core_t* core, xce_core_event_t* events,
                      * FREEUP(job) below only frees the job shell itself, not
                      * these two fields, matching this function's existing
                      * "job->allocs/accounts are out of scope" convention. */
+                    /* bug-1884: free what the job owns, not only the
+                     * containers */
+                    xce_core_free_schedobj(job->schedobj);
+                    kill_resreq(job->resreq);
                     FREEUP(job->resreq);
-                    FREEUP(job->schedobj);
                     FREEUP(job);
                     continue;
                 }
@@ -1374,6 +1379,8 @@ int xce_core_push_events(xce_sched_core_t* core, xce_core_event_t* events,
                         core_rebind_schedobj_resreq(job->schedobj, newreq);
                     }
 
+                    /* bug-1884: the schedobj now points at newreq */
+                    kill_resreq(job->resreq);
                     FREEUP(job->resreq);
                     job->resreq = newreq;
                 }

src/scheduler/src/recording.c

@@ -354,6 +354,27 @@ static int write_replay_header(FILE* f, xce_scheduler_t* sched,
  * save_snapshot() result unfreed for the same reason, detect_leaks=0
  * across this whole suite) -- writing one is out of this task's boundary.
  */
+/* sched_snapshot_data_free - bug-1884: frees what
+ * xce_sched_core_save_snapshot() allocated in `snap` (not `snap` itself).
+ * The config and the resource objects are the core's (borrowed, see
+ * snapshot.c's field notes), so they are detached first. */
+static void sched_snapshot_data_free(sched_snapshot_data_t* snap)
+{
+    int i;
+
+    if (snap == NULL) {
+        return;
+    }
+
+    snap->config = NULL;
+    if (snap->resources != NULL) {
+        for (i = 0; i < snap->resources->count; i++) {
+            snap->resources->resources[i] = NULL;
+        }
+    }
+    free_SnapshotData(snap);
+}
+
 int write_snapshot_entry(FILE* f, xce_sched_core_t* core, uint64_t ts)
 {
     struct capn             c;
@@ -382,6 +403,8 @@ int write_snapshot_entry(FILE* f, xce_sched_core_t* core, uint64_t ts)
     rc = write_capnp_message(f, &c);
     capn_free(&c);
 
+    sched_snapshot_data_free(&snap);
+
     return rc;
 }
 

src/scheduler/src/resource.c

@@ -98,6 +98,19 @@ int put_resvals(zarray_t* array, int nres, int* idxs, float* vals)
             val->u.fvalue = 0.0;
             zarray_put(array, idxs[i], val);
             xvals[i] = val;
+
+            /* bug-1884: a later entry for the same index shares this value;
+             * creating a second one replaced (and, with the array's killer,
+             * freed) this one while xvals[i] still pointed at it */
+            {
+                int k;
+
+                for (k = i + 1; k < nres; k++) {
+                    if ((idxs[k] == idxs[i]) && (xvals[k] == NULL)) {
+                        xvals[k] = val;
+                    }
+                }
+            }
         }
     }
 

src/scheduler/src/schedobj.c

@@ -276,13 +276,18 @@ void kill_sched_worker(void* arg)
     sched_worker_release_host_ctx((xce_sched_worker_t*)arg);
 }
 
+/* kill_sched_resval - item killer of every resource-value array. bug-1884:
+ * frees the value itself too -- free_Value() frees only a string value's
+ * buffer, so every replaced or torn-down value leaked its struct. All
+ * values stored in these arrays are separately allocated (the wire
+ * decoder, dup_val(), put_resvals()). */
 void kill_sched_resval(xce_val_t* val)
 {
     if (val == NULL) {
         return;
     }
 
-    free_Value(val);
+    free_Value_ptr(&val);
 }
 
 xce_sched_worker_t* make_sched_worker(void* arg, xce_worker_t* w)

src/scheduler/src/scheduler.c

@@ -322,6 +322,9 @@ static int sched_make_primary_allocation(xce_sched_core_t* core,
     }
 
     if (ctx.reason != NULL) {
+        /* bug-1884: the previous attempt's reason (its decision has
+         * been published) or an earlier alternative's */
+        free_PendReason_ptr(&(obj->reason));
         MOVE_PTR(obj->reason, ctx.reason);
     }
 
@@ -386,6 +389,9 @@ static int sched_make_compound_alloc_(xce_sched_core_t*      core,
     }
 
     if (ctx.reason != NULL) {
+        /* bug-1884: the previous attempt's reason (its decision has
+         * been published) or an earlier alternative's */
+        free_PendReason_ptr(&(obj->reason));
         MOVE_PTR(obj->reason, ctx.reason);
     }
 

src/scheduler/src/snapshot.c

@@ -446,7 +446,7 @@ int xce_sched_core_save_snapshot(xce_sched_core_t*      core,
         if (rv == NULL) {
             continue;
         }
-        rv->name = makeString((char*)rname);
+        rv->name = strdup(rname);
         rv->host = NULL;
         rv->val  = dup_val(val);
 
@@ -548,7 +548,7 @@ int xce_sched_core_save_snapshot(xce_sched_core_t*      core,
             if (su == NULL) {
                 continue;
             }
-            su->name = (u->name != NULL) ? makeString(u->name) : NULL;
+            su->name = (u->name != NULL) ? strdup(u->name) : NULL;
 
             if (arr_append((void***)&users_arr, &users_cap, &users_n, su) < 0) {
                 FREEUP(su->name);
@@ -575,7 +575,7 @@ int xce_sched_core_save_snapshot(xce_sched_core_t*      core,
             if (sp == NULL) {
                 continue;
             }
-            sp->name     = (p->name != NULL) ? makeString(p->name) : NULL;
+            sp->name     = (p->name != NULL) ? strdup(p->name) : NULL;
             sp->priority = p->priority;
 
             if (arr_append((void***)&proj_arr, &proj_cap, &proj_n, sp) < 0) {

src/scheduler/src/snapshot_wire.c

@@ -148,10 +148,10 @@ xce_job_t* build_wire_job(xce_sched_job_t* job)
     }
 
     j->submit->queue
-        = (job->queue != NULL) ? makeString(job->queue->name) : NULL;
+        = (job->queue != NULL) ? strdup(job->queue->name) : NULL;
-    j->submit->user = (job->user != NULL) ? makeString(job->user->name) : NULL;
+    j->submit->user = (job->user != NULL) ? strdup(job->user->name) : NULL;
     j->submit->project_name
-        = (job->project != NULL) ? makeString(job->project->name) : NULL;
+        = (job->project != NULL) ? strdup(job->project->name) : NULL;
     j->submit->mintasks = job->mintasks;
     j->submit->maxtasks = job->maxtasks;
 
@@ -206,13 +206,13 @@ xce_queue_t* build_wire_queue(xce_sched_queue_t* q)
         return NULL;
     }
 
-    wq->queue           = (q->name != NULL) ? makeString(q->name) : NULL;
+    wq->queue           = (q->name != NULL) ? strdup(q->name) : NULL;
     wq->attrs           = q->attrs;
     wq->status          = q->status;
     wq->priority        = q->priority;
     wq->accept_interval = q->accept_interval;
     wq->sched_delay     = q->sched_delay;
-    wq->resreq          = (q->resreq != NULL) ? makeString(q->resreq) : NULL;
+    wq->resreq          = (q->resreq != NULL) ? strdup(q->resreq) : NULL;
 
     /* project_name/job_pick_plugin: no wire slot -- see snapshot.c header
      * "Scope decisions". */
@@ -238,7 +238,7 @@ xce_worker_t* build_wire_worker(xce_sched_worker_t* w,
         return NULL;
     }
 
-    ww->name   = (w->name != NULL) ? makeString(w->name) : NULL;
+    ww->name   = (w->name != NULL) ? strdup(w->name) : NULL;
     ww->status = w->status;
 
     for (i = 0; i < w->resvals.count; i++) {
@@ -275,7 +275,7 @@ xce_worker_t* build_wire_worker(xce_sched_worker_t* w,
             continue;
         }
 
-        nv->name = makeString((char*)rname);
+        nv->name = strdup(rname);
         nv->val  = dup_val(val);
 
         if (arr_append((void***)&nvs, &cap, &n, nv) < 0) {

src/scheduler/testsuite/test_sim_discrete_clock.c

@@ -1245,6 +1245,33 @@ static void test_cli_help_describes_sim_speed_as_deprecated(void)
     free(text);
 }
 
+/* bug-1884: a batch replay frees what the simulator owns before exit, so an
+ * ASAN build with LeakSanitizer enabled exits 0. The test binary itself runs
+ * with detect_leaks=0 (meson.build), so the child re-enables it. Without
+ * ASAN the option is ignored and the test checks only the exit status. */
+static void test_cli_batch_replay_exits_clean_under_leak_detection(void)
+{
+    char fixture[PATH_MAX];
+    char out[PATH_MAX];
+    char log[PATH_MAX];
+    char cmd[4 * PATH_MAX];
+
+    write_two_session_fixture(fixture, sizeof(fixture));
+    make_tmp_file(out, PATH_MAX, "leakout");
+    make_tmp_file(log, PATH_MAX, "leaklog");
+    snprintf(cmd, sizeof(cmd),
+             "ASAN_OPTIONS=detect_leaks=1 " XCE_SCHEDULER_BIN
+             " --sim %s --sim-verify --sim-output %s --logfile %s "
+             "> /dev/null 2>&1",
+             fixture, out, log);
+    TEST_ASSERT_EQUAL_INT_MESSAGE(0, run_child(cmd),
+                                  "a clean batch replay must exit 0 with "
+                                  "leak detection enabled");
+    unlink(out);
+    unlink(log);
+    unlink(fixture);
+}
+
 int main(void)
 {
     UNITY_BEGIN();
@@ -1265,5 +1292,6 @@ int main(void)
     RUN_TEST(test_cli_trailing_sim_speed_without_value_fails);
     RUN_TEST(test_cli_interactive_sim_speed_logs_one_warning);
     RUN_TEST(test_cli_help_describes_sim_speed_as_deprecated);
+    RUN_TEST(test_cli_batch_replay_exits_clean_under_leak_detection);
     return UNITY_END();
 }