From c95bc8712edceaa8531df68494139727aadee2c6 Mon Sep 17 00:00:00 2001 From: daywalker90 Date: Tue, 11 Aug 2026 14:39:40 +0200 Subject: [PATCH] wire_sync: tolerate a non-blocking fd when reading/writing a message On macOS under load, subdaemons intermittently die: connectd: **BROKEN** STATUS_FAIL_HSM_IO: No hsmd ECDH response channeld: exits 0 after WIRE_HSMD_GET_PER_COMMITMENT_POINT Both are a synchronous wire_sync_read() returning NULL on a fresh connection. The HSM fd in a subdaemon is one end of a socketpair created by hsmd (whose io loop sets O_NONBLOCK on the other end) and passed hsmd -> lightningd -> subdaemon via SCM_RIGHTS. On macOS the O_NONBLOCK flag follows the shared open file description across that chain, so the subdaemon's fd can be non-blocking, and read()/write() return EAGAIN before hsmd's (fast) reply has landed. Linux keeps the descriptions independent, which is why this never reproduces there. The previous attempts to force the fd blocking from the subdaemon (io_fd_block in ecdh_hsmd_setup) cannot win, because hsmd owns the other end of the same open file description and keeps it non-blocking. Fix at the shared choke point instead: make wire_sync_read() and wire_sync_write() tolerant of O_NONBLOCK by polling on EAGAIN and resuming, preserving any partial read. "Sync" then really means "read or write a complete message", regardless of the fd's blocking state. This covers connectd's ecdh(), channeld's hsm_req(), and every other subdaemon that does synchronous HSM I/O (openingd, closingd, onchaind). Also drop the now-unneeded io_fd_block() toggling from ecdh_hsmd_setup(), and include errno in the HSM I/O failure messages so a recurrence is diagnosable from the daemon log. Changelog-Fixed: connectd: fix intermittent "No hsmd ECDH response" crash on macOS under load (issue #9060). --- common/ecdh_hsmd.c | 17 +++++++---- wire/wire_sync.c | 74 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/common/ecdh_hsmd.c b/common/ecdh_hsmd.c index 68be59841..c55a4a60d 100644 --- a/common/ecdh_hsmd.c +++ b/common/ecdh_hsmd.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -13,16 +14,22 @@ static void (*stashed_failed)(enum status_failreason, const char *fmt, ...); void ecdh(const struct pubkey *point, struct secret *ss) { const u8 *msg = towire_hsmd_ecdh_req(NULL, point); + u8 *resp; assert(stashed_hsm_fd >= 0); assert(stashed_failed != NULL); + /* wire_sync_read/write tolerate a non-blocking fd (the HSM socketpair + * can be O_NONBLOCK on macOS), so no blocking toggling is needed here. + * Report errno so a failure is diagnosable from the daemon log. */ if (!wire_sync_write(stashed_hsm_fd, take(msg))) - stashed_failed(STATUS_FAIL_HSM_IO, "Write ECDH to hsmd failed"); + stashed_failed(STATUS_FAIL_HSM_IO, "Write ECDH to hsmd failed: %s", + strerror(errno)); - msg = wire_sync_read(tmpctx, stashed_hsm_fd); - if (!msg) - stashed_failed(STATUS_FAIL_HSM_IO, "No hsmd ECDH response"); + resp = wire_sync_read(tmpctx, stashed_hsm_fd); + if (!resp) + stashed_failed(STATUS_FAIL_HSM_IO, "No hsmd ECDH response: %s", + strerror(errno)); if (!fromwire_hsmd_ecdh_resp(msg, ss)) stashed_failed(STATUS_FAIL_HSM_IO, "Invalid hsmd ECDH response"); @@ -34,6 +41,4 @@ void ecdh_hsmd_setup(int hsm_fd, { stashed_hsm_fd = hsm_fd; stashed_failed = failed; - /* Like read_fds in subd.c: don't trust sender's O_NONBLOCK state (issue #9060). */ - io_fd_block(hsm_fd, true); } diff --git a/wire/wire_sync.c b/wire/wire_sync.c index 954515136..dfc3d592d 100644 --- a/wire/wire_sync.c +++ b/wire/wire_sync.c @@ -1,19 +1,83 @@ #include "config.h" #include -#include #include #include +#include #include #include +/* Wait until fd is ready (readable or writable), tolerating EINTR. */ +static bool wait_fd(int fd, short events) +{ + int r; + + do { + r = poll(&(struct pollfd){.fd = fd, .events = events}, 1, -1); + } while (r < 0 && errno == EINTR); + + return r > 0; +} + +/* Like read_all, but tolerates an fd with O_NONBLOCK set: on EAGAIN we poll + * for readability and resume, preserving any partial read. Subdaemon HSM fds + * can be O_NONBLOCK on macOS (the flag follows the shared open file + * description as the socketpair is sent via SCM_RIGHTS), so without this we + * would spuriously fail with EAGAIN. */ +static bool read_all_tolerant(int fd, void *buf, size_t size) +{ + while (size) { + ssize_t done = read(fd, buf, size); + if (done < 0) { + if (errno == EINTR) + continue; + if (errno == EAGAIN || errno == EWOULDBLOCK) { + if (!wait_fd(fd, POLLIN)) + return false; + continue; + } + return false; + } + if (done == 0) + return false; + buf = (char *)buf + done; + size -= done; + } + + return true; +} + +/* Mirror of read_all_tolerant for writes. */ +static bool write_all_tolerant(int fd, const void *buf, size_t size) +{ + while (size) { + ssize_t done = write(fd, buf, size); + if (done < 0) { + if (errno == EINTR) + continue; + if (errno == EAGAIN || errno == EWOULDBLOCK) { + if (!wait_fd(fd, POLLOUT)) + return false; + continue; + } + return false; + } + if (done == 0) + return false; + buf = (const char *)buf + done; + size -= done; + } + + return true; +} + bool wire_sync_write(int fd, const void *msg TAKES) { wire_len_t hdr = cpu_to_wirelen(tal_bytelen(msg)); bool ret; assert(tal_bytelen(msg) < WIRE_LEN_LIMIT); - ret = write_all(fd, &hdr, sizeof(hdr)) - && write_all(fd, msg, tal_count(msg)); + ret = write_all_tolerant(fd, &hdr, sizeof(hdr)) + && write_all_tolerant(fd, msg, tal_count(msg)); tal_free_if_taken(msg); return ret; @@ -24,14 +88,14 @@ u8 *wire_sync_read(const tal_t *ctx, int fd) wire_len_t len; u8 *msg; - if (!read_all(fd, &len, sizeof(len))) + if (!read_all_tolerant(fd, &len, sizeof(len))) return NULL; if (wirelen_to_cpu(len) >= WIRE_LEN_LIMIT) { errno = E2BIG; return NULL; } msg = tal_arr(ctx, u8, wirelen_to_cpu(len)); - if (!read_all(fd, msg, wirelen_to_cpu(len))) + if (!read_all_tolerant(fd, msg, wirelen_to_cpu(len))) return tal_free(msg); return msg; }