From ffc2c590f9437bb1ca939d6e91ddf4bb236e2070 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 17 Jun 2026 06:53:46 +0000 Subject: [PATCH] =?UTF-8?q?fix(libxce-failover):=20correct=20endpoint=20re?= =?UTF-8?q?connect=20on=20rotate=20=E2=80=94=20re-bind=20iomgr=20+=20re-ar?= =?UTF-8?q?m=20handlers=20(fixes=20endpoint=5Fconnect=20SEGV=20during=20li?= =?UTF-8?q?ve=20failover)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xce_failover_rotate reconnected with endpoint_fini+endpoint_init+endpoint_connect but never re-bound ep->iomgr (reset by endpoint_init) nor re-armed the owner/ msg/err handlers + iomgr_monitor. endpoint_connect then dereferenced a NULL iomgr (->ssl_ctx) → SEGV at netio.c:672 during live master failover. Added xce_ctx_reconnect() in xcectx.c that mirrors xcecli_start's establishment (without restarting iomgr/vtaskmgr or touching ctx->state); rotate delegates to it. Found via live 3-node E2E. bug-610. --- .wolf/buglog.json | 24 ++++++++++++++++ src/include/xcectx.h | 1 + src/lib/client_failover.c | 29 ++++++++----------- src/lib/xcectx.c | 59 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 18 deletions(-) diff --git a/.wolf/buglog.json b/.wolf/buglog.json index 0fa5c8d4..c95a572f 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -10446,6 +10446,30 @@ ], "occurrences": 1, "last_seen": "2026-06-17T04:45:00Z" + }, + { + "id": "bug-610", + "timestamp": "2026-06-17T15:00:00Z", + "error_message": "ASan SEGV in endpoint_connect (netio.c:672) via xce_failover_rotate \u2192 xcecli_retry_next \u2192 xcecli_joblist during live master failover (NULL ep->iomgr deref)", + "file": "src/lib/client_failover.c", + "root_cause": "xce_failover_rotate reconnected via endpoint_fini + endpoint_init + endpoint_connect, but endpoint_init (do_endpoint_init) resets the endpoint struct including ep->iomgr, and the rotate never re-bound it. The client's endpoint gets ep->iomgr = &ctx->iomgr only via xce_ctx_set_endpoint (called once in xcecli_start). endpoint_connect derefs iomgr->ssl_ctx when ep->ssl==NULL (true after endpoint_fini frees the SSL) \u2192 NULL deref crash. Also the rotate never re-set owner/msg_handler/err_handler nor iomgr_monitor'd the new fd, so even without the crash the reconnected endpoint would not route replies. Pure unit tests could not exercise the live socket/iomgr path; only the live failover E2E hit it.", + "fix": "Added xce_ctx_reconnect(ctx,host,port) in xcectx.c (where static handle_msg/handle_disconnect live) that mirrors the connection-establishment of xcecli_start/xce_ctx_start without restarting iomgr/vtaskmgr or touching ctx->state: endpoint_fini \u2192 endpoint_init \u2192 endpoint_set_peer \u2192 re-bind ep->iomgr=&ctx->iomgr \u2192 endpoint_connect \u2192 re-set owner+msg/err handlers \u2192 iomgr_monitor. xce_failover_rotate now delegates the reconnect to it.", + "tags": [ + "libxce", + "failover", + "crash", + "SEGV", + "endpoint_connect", + "iomgr", + "reconnect", + "E2E" + ], + "related_bugs": [ + "bug-609", + "bug-608" + ], + "occurrences": 1, + "last_seen": "2026-06-17T15:00:00Z" } ] } \ No newline at end of file diff --git a/src/include/xcectx.h b/src/include/xcectx.h index 424c95e6..7e321778 100644 --- a/src/include/xcectx.h +++ b/src/include/xcectx.h @@ -56,6 +56,7 @@ extern int xce_ctx_stop(xce_ctx_t*); extern int xce_wakeup(xce_ctx_t*, int); extern int xce_try_wakeup(xce_ctx_t*, int); extern int xce_ctx_set_endpoint(xce_ctx_t*, endpoint_t*); +extern int xce_ctx_reconnect(xce_ctx_t*, char*, int); extern void xce_block_tasks(xce_ctx_t*); extern void xce_unblock_tasks(xce_ctx_t*); diff --git a/src/lib/client_failover.c b/src/lib/client_failover.c index 8f51e66f..ab85f522 100644 --- a/src/lib/client_failover.c +++ b/src/lib/client_failover.c @@ -139,24 +139,17 @@ int xce_failover_rotate(api_ctx_t *ac) if (rc < 0) return rc; - /* Close the stale connection FIRST, then reconnect the SAME endpoint - * object so reply routing to ctx->wbuf (via ctx->ep == &ac->ep) is - * preserved. endpoint_fini() closes the fd, frees the SSL session, and - * clears handlers/peer; endpoint_init() recreates the socket fd and - * re-arms the object before endpoint_connect() drives the handshake. - * - * The live socket/handshake path is exercised by the E2E task (6.2); this - * wrapper keeps the I/O surface thin around the pure selector above. */ - endpoint_fini(&ac->ep); - - rc = endpoint_init(&ac->ep, AF_INET, SOCK_STREAM, 0, 0); - if (rc < 0) - return rc; - - endpoint_set_peer(&ac->ep, &ac->ctx.myinfo); - - rc = endpoint_connect(&ac->ep, host, (int)port); - return rc < 0 ? rc : 0; + /* Re-establish the SAME endpoint object on a fresh connection to the chosen + * target. xce_ctx_reconnect() (xcectx.c) owns the full connection setup that + * endpoint_fini drops — close old fd/SSL, recreate the socket, RE-BIND the + * iomgr (endpoint_connect needs ep->iomgr->ssl_ctx — a missing re-bind here + * previously crashed in endpoint_connect), drive the SSL handshake, and + * re-arm the endpoint in the running iomgr with its owner + msg/err handlers + * and re-monitor the new fd so replies route to ctx->wbuf and RDHUP failover + * re-arms. It does not restart iomgr/vtaskmgr or touch ctx->state (the loop's + * xcecli_ctx_recover owns the ERR→RUN reset). The live socket/handshake path + * is exercised by the E2E task (6.2). */ + return xce_ctx_reconnect(&ac->ctx, host, (int)port); } /* ════════════════════════════════════════════════════════════════════════ diff --git a/src/lib/xcectx.c b/src/lib/xcectx.c index 6978f161..278ed5dd 100644 --- a/src/lib/xcectx.c +++ b/src/lib/xcectx.c @@ -301,6 +301,65 @@ int xce_ctx_start(xce_ctx_t* ctx) return 0; } +/* + * xce_ctx_reconnect (libxce-failover) — re-establish ctx->ep on a fresh + * connection to host:port after the previous primary's connection dropped. + * + * This mirrors the connection-establishment part of xcecli_start/xce_ctx_start + * (the parts that DROP on endpoint_fini): tear down the dead fd/SSL, recreate + * the socket, re-bind the iomgr (endpoint_connect needs ep->iomgr->ssl_ctx), + * drive the SSL handshake, then re-arm the endpoint in the ALREADY-RUNNING + * iomgr with its owner + msg/err handlers and re-monitor the new fd. It does + * NOT restart iomgr/vtaskmgr (already running) and does NOT touch ctx->state + * (the failover loop's xcecli_ctx_recover owns the ERR→RUN reset). + * + * Without the iomgr re-bind, endpoint_connect dereferenced a NULL ep->iomgr + * (crash at netio.c endpoint_connect); without re-arming the handlers + monitor + * the reconnected endpoint would never route replies to ctx->wbuf. + */ +int xce_ctx_reconnect(xce_ctx_t* ctx, char* host, int port) +{ + int rc; + + if ((ctx == NULL) || (ctx->ep == NULL) || (host == NULL)) { + return -EINVAL; + } + + /* Tear down the dead connection (closes fd, frees SSL, clears handlers and + * resets the endpoint struct — including ep->iomgr). */ + endpoint_fini(ctx->ep); + + rc = endpoint_init(ctx->ep, AF_INET, SOCK_STREAM, 0, 0); + if (rc < 0) { + return rc; + } + + endpoint_set_peer(ctx->ep, &(ctx->myinfo)); + + /* Re-bind the iomgr so endpoint_connect can reach ctx->iomgr->ssl_ctx + * (xce_ctx_set_endpoint does this on the initial start). */ + ctx->ep->iomgr = &(ctx->iomgr); + + rc = endpoint_connect(ctx->ep, host, port); + if (rc < 0) { + return rc; + } + + /* Re-arm the endpoint in the running iomgr: owner + reply/error handlers, + * then monitor the new fd. handle_disconnect re-registers RDHUP-driven + * failover for the new primary. */ + endpoint_set_owner(ctx->ep, ctx); + endpoint_set_msg_handler(ctx->ep, (msg_handler_t)handle_msg); + endpoint_set_err_handler(ctx->ep, (err_handler_t)handle_disconnect); + + rc = iomgr_monitor(ctx->ep); + if (rc < 0) { + return rc; + } + + return 0; +} + int xce_ctx_stop(xce_ctx_t* ctx) { if (ctx == NULL) {