fix(libxce): swallow nop reply so heartbeat can't poison shared wbuf (bug-1508)

The apiserver's 30s heartbeat sends a fire-and-forget nop (xce_nop) that
never waits for the reply. The controller replies to every nop, and
xce_handle_reply's default case returned -ENOENT for OPCode_nop, so
vtask_entry called xce_wakeup and left the nop's status sitting in the
single-slot wbuf. The next blocking request (xce_fetchfile / xce_jobsub
/ xce_jobctrl) then consumed that stale status as its own reply:
the fetchfile waiter woke with status=0 but ctx->reply.u.fetchfilereply
still NULL -> 'nil reply' -> job peek 501.

Return 0 for OPCode_nop replies so they never touch wbuf. Control-op
replies (jobctrl/queuectrl/hostctrl) legitimately hit the default case
and still wake.

Regression test: test_handle_reply_nop_does_not_wake (red->green).
This commit is contained in:
lei.chen 2026-08-05 09:41:00 +00:00
parent c091a8dae0
commit 730a8ebcd4
2 changed files with 50 additions and 0 deletions

View file

@ -446,10 +446,46 @@ static void test_handle_reply_ctrlstatus_does_not_free_foreign_slot(void)
printf("test_handle_reply_ctrlstatus_does_not_free_foreign_slot: PASS\n"); printf("test_handle_reply_ctrlstatus_does_not_free_foreign_slot: PASS\n");
} }
/* bug-15xx (job peek 501 "nil reply"): a fire-and-forget nop/heartbeat reply
* must NOT wake the shared single-slot wbuf — xce_handle_reply returning 0 here
* is what keeps vtask_entry from calling xce_wakeup, so the stale nop status can
* never be consumed by the next blocking request as its own reply. Also proves a
* foreign (poisoned) union slot is left untouched, mirroring the bug-1077
* ctrlstatus assertion. */
static void test_handle_reply_nop_does_not_wake(void)
{
xce_ctx_t ctx;
memset(&ctx, 0, sizeof(ctx));
/* A foreign occupant in the shared reply union must survive a nop reply
* untouched (no free, no deref) — same discipline as bug-1077. */
xce_fetchfile_reply_t* sentinel = (xce_fetchfile_reply_t*)calloc(1, sizeof(*sentinel));
TEST_ASSERT_NOT_NULL(sentinel);
ctx.reply.u.fetchfilereply = sentinel; /* same 8 bytes as any other slot */
xce_reply_t reply;
memset(&reply, 0, sizeof(reply));
reply.opcode = OPCode_nop;
reply.status = 0;
reply.data_which = RequestReply_data_nodata;
int rc = xce_handle_reply(&ctx, &reply);
/* rc==0 is the contract vtask_entry relies on to skip xce_wakeup: the nop
* reply must not become a pending status in wbuf. */
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_PTR(sentinel, ctx.reply.u.fetchfilereply);
free(sentinel);
printf("test_handle_reply_nop_does_not_wake: PASS\n");
}
/* ── Suite entry point ──────────────────────────────────────────────────── */ /* ── Suite entry point ──────────────────────────────────────────────────── */
void test_ctrlstatus(void) void test_ctrlstatus(void)
{ {
RUN_TEST(test_handle_reply_nop_does_not_wake);
RUN_TEST(test_handle_reply_ctrlstatus_does_not_free_foreign_slot); RUN_TEST(test_handle_reply_ctrlstatus_does_not_free_foreign_slot);
RUN_TEST(test_ctrlstatus_roundtrip_all_fields); RUN_TEST(test_ctrlstatus_roundtrip_all_fields);
RUN_TEST(test_ctrlstatus_roundtrip_unknown_primary); RUN_TEST(test_ctrlstatus_roundtrip_unknown_primary);

View file

@ -69,6 +69,20 @@ int xce_handle_reply(xce_ctx_t* ctx, xce_reply_t* reply)
xce_ctx_capture_redirect_hint(ctx, reply->redirect_host, xce_ctx_capture_redirect_hint(ctx, reply->redirect_host,
reply->redirect_port); reply->redirect_port);
/* bug-15xx (job peek 501 "nil reply"): a nop/heartbeat reply is fire-and-
* forget on the send side — xce_nop/xce_heartbeat sends it and never waits
* for the reply. Waking the shared single-slot wbuf here therefore leaves a
* stale status sitting in the buffer that the NEXT blocking request
* (xce_fetchfile / xce_jobsub / xce_jobctrl ...) consumes as its own reply:
* the fetchfile waiter wakes with status=0 but ctx->reply.u.fetchfilereply
* still NULL -> "nil reply" -> 501. Swallow nop replies entirely so they
* never touch wbuf; the reply itself is freed by the caller's free_Request.
* (The control-op replies -- jobctrl/queuectrl/hostctrl -- legitimately hit
* the default case below and MUST wake: their callers block on wbuf.) */
if (reply->opcode == OPCode_nop) {
return 0;
}
if (reply->status < 0) { if (reply->status < 0) {
return reply->status; return reply->status;
} }