fix(controller): never close() the primary channel's raw fd (bug-1865)
handle_ctrlhello stored the netio endpoint's fd (primary_ctrl_fd = ep->fd) and ha_on_primary_ctrl_fd_rdhup close()d it in three branches. netio owns that socket and closes it on endpoint teardown, so by the time the liveness poll fired the RDHUP the number could be reused -- in a chaotic soak by the standby's WAL segment: HA's close() shut the WAL, fdatasync failed 2305 times (EBADF, then EINVAL once a socket took the number), durable_lsn stuck, and one round's jobsub failed EIO. handle_ctrlhello now also holds a reference on the endpoint (primary_ctrl_ep), which keeps its socket open and its fd number ours. ha_drop_primary_ctrl_conn() replaces the raw closes: shutdown() the still-owned socket so netio sees the hang-up and tears it down itself, then release the reference. A bare fd with no endpoint is only forgotten. Test test_rdhup_never_closes_an_fd_netio_already_released: real ctrlhello -> netio teardown -> file opened on the freed number -> RDHUP; the file was closed before, survives now. test_rdhup_closes_and_nulls_fd updated to the new contract (peer reads EOF, fields cleared). Controller suite 5/5. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
98c52b7d96
commit
910b43cd37
5 changed files with 127 additions and 22 deletions
|
|
@ -12,6 +12,9 @@
|
|||
*/
|
||||
#define _GNU_SOURCE
|
||||
#include <errno.h>
|
||||
#include <fcntl.h>
|
||||
#include <sys/socket.h>
|
||||
#include <unistd.h>
|
||||
#include <stdatomic.h>
|
||||
#include <stdbool.h>
|
||||
#include <stdio.h>
|
||||
|
|
@ -427,6 +430,67 @@ static void test_handle_ctrlhello_sets_primary_ctrl_fd(void)
|
|||
printf("test_handle_ctrlhello_sets_primary_ctrl_fd: PASS\n");
|
||||
}
|
||||
|
||||
/* bug-1865: handle_ctrlhello kept a copy of the netio endpoint's raw fd and
|
||||
* ha_on_primary_ctrl_fd_rdhup close()d it -- but netio owns that socket and
|
||||
* closes it on endpoint teardown. By the time the liveness poll fired the
|
||||
* RDHUP, the number had been reused by the standby's WAL segment, and HA's
|
||||
* close() shut the WAL instead (fdatasync EBADF/EINVAL, durable_lsn stuck).
|
||||
* Drive the real path: ctrlhello records the heap endpoint, netio drops its
|
||||
* own reference (teardown), a file is opened, then RDHUP -- the file must
|
||||
* survive. */
|
||||
extern void ha_on_primary_ctrl_fd_rdhup(ha_state_t *ha, cluster_t *c);
|
||||
|
||||
static void test_rdhup_never_closes_an_fd_netio_already_released(void)
|
||||
{
|
||||
cluster_t c;
|
||||
myconf_t conf;
|
||||
ctrl_hello_req_t hello;
|
||||
xce_request_t req;
|
||||
endpoint_t* ep;
|
||||
int sv[2];
|
||||
int file_fd;
|
||||
|
||||
memset(&c, 0, sizeof(c));
|
||||
ha_init(&c.ha, &g_cfg, 0);
|
||||
c.ha.primary_ctrl_fd = -1;
|
||||
atomic_store_explicit(&c.ha.role, (int32_t)CTRL_STANDBY,
|
||||
memory_order_release);
|
||||
memset(&conf, 0, sizeof(conf));
|
||||
c.myconf = &conf;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, socketpair(AF_UNIX, SOCK_STREAM, 0, sv));
|
||||
ep = endpoint_get(); /* heap ep, netio's own reference */
|
||||
TEST_ASSERT_NOT_NULL(ep);
|
||||
ep->fd = sv[0];
|
||||
|
||||
memset(&hello, 0, sizeof(hello));
|
||||
hello.name = "primary";
|
||||
memset(&req, 0, sizeof(req));
|
||||
req.body_which = Request_body_ctrlhellomsg;
|
||||
req.body.ctrlhellomsg = &hello;
|
||||
(void)handle_ctrlhello(&c, ep, &req);
|
||||
TEST_ASSERT_EQUAL_INT(sv[0], c.ha.primary_ctrl_fd);
|
||||
|
||||
/* netio tears the connection down (drops its reference) */
|
||||
endpoint_unref(ep);
|
||||
|
||||
/* the standby then opens its WAL segment: lowest free fd */
|
||||
file_fd = open("/dev/null", O_RDONLY);
|
||||
TEST_ASSERT_TRUE(file_fd >= 0);
|
||||
|
||||
ha_on_primary_ctrl_fd_rdhup(&c.ha, NULL);
|
||||
|
||||
TEST_ASSERT_TRUE_MESSAGE(fcntl(file_fd, F_GETFD) != -1,
|
||||
"RDHUP must not close an fd that netio released -- here it had been "
|
||||
"reused by an unrelated file (the WAL in production)");
|
||||
TEST_ASSERT_EQUAL_INT(-1, c.ha.primary_ctrl_fd);
|
||||
|
||||
close(file_fd);
|
||||
close(sv[1]);
|
||||
ha_fini(&c.ha);
|
||||
printf("test_rdhup_never_closes_an_fd_netio_already_released: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Test 8b (bug-1701): handle_ctrlhello populates primary_host/port from
|
||||
* the sender's name matched against our own static peers_cfg candidate list.
|
||||
* This is the local-lookup fix that unblocks bug-1700's fill_redirect_reply()
|
||||
|
|
@ -631,6 +695,7 @@ void test_ha_connection_reuse(void)
|
|||
|
||||
/* Req 3.1 — handle_ctrlhello designates primary control channel */
|
||||
RUN_TEST(test_handle_ctrlhello_sets_primary_ctrl_fd);
|
||||
RUN_TEST(test_rdhup_never_closes_an_fd_netio_already_released);
|
||||
|
||||
/* bug-1701 — handle_ctrlhello populates primary_host/primary_port via a
|
||||
* local peers_cfg lookup keyed by req->from->name */
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@
|
|||
|
||||
#include <unity/unity.h>
|
||||
#include "ha.h"
|
||||
#include "netio.h"
|
||||
#include "xce/cluster_def.h" /* full cluster_t layout for task-5.2 tests */
|
||||
#include "myconf.h" /* myconf_t for task-5.2 tests */
|
||||
|
||||
|
|
@ -91,28 +92,38 @@ static ha_config_t g_fast_cfg = {
|
|||
.election_priority_step_ms = 500,
|
||||
};
|
||||
|
||||
/* ── Test 1: RDHUP closes fd and sets primary_ctrl_fd = -1 ─────────────── */
|
||||
/* ── Test 1: RDHUP drops the primary channel and sets primary_ctrl_fd = -1 ──
|
||||
* bug-1865: the channel is a netio endpoint HA holds a reference on; RDHUP
|
||||
* shuts its socket down (the peer sees EOF) and releases the reference --
|
||||
* it never close()s the raw fd, which netio owns. */
|
||||
|
||||
static void test_rdhup_closes_and_nulls_fd(void)
|
||||
{
|
||||
ha_state_t ha;
|
||||
ha_state_t ha;
|
||||
endpoint_t* ep;
|
||||
char b;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_cfg, 1));
|
||||
|
||||
/* Set up a socketpair as the "primary_ctrl_fd" */
|
||||
/* Set up a socketpair as the primary channel, held like handle_ctrlhello */
|
||||
int sv[2];
|
||||
TEST_ASSERT_EQUAL_INT(0, socketpair(AF_UNIX, SOCK_STREAM, 0, sv));
|
||||
ep = endpoint_get();
|
||||
TEST_ASSERT_NOT_NULL(ep);
|
||||
ep->fd = sv[0];
|
||||
endpoint_ref(ep);
|
||||
ha.primary_ctrl_fd = sv[0];
|
||||
ha.primary_ctrl_ep = ep;
|
||||
|
||||
/* Fire RDHUP with NULL cluster (unit test mode — skips persist/election) */
|
||||
ha_on_primary_ctrl_fd_rdhup(&ha, NULL);
|
||||
|
||||
/* fd must be closed and set to -1 */
|
||||
/* channel dropped: fields cleared, peer sees the hang-up */
|
||||
TEST_ASSERT_EQUAL_INT(-1, ha.primary_ctrl_fd);
|
||||
/* Verify sv[0] is really closed by trying to write to it */
|
||||
ssize_t n = write(sv[0], "x", 1);
|
||||
TEST_ASSERT_LESS_THAN_INT(0, (int)n);
|
||||
TEST_ASSERT_EQUAL_INT(EBADF, errno);
|
||||
TEST_ASSERT_NULL(ha.primary_ctrl_ep);
|
||||
TEST_ASSERT_EQUAL_INT(0, (int)read(sv[1], &b, 1));
|
||||
|
||||
endpoint_unref(ep); /* netio's own reference: closes sv[0] */
|
||||
close(sv[1]);
|
||||
ha_fini(&ha);
|
||||
printf("test_rdhup_closes_and_nulls_fd: PASS\n");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue