Skip to content

document the accepted-socket set_nonblocking(false) fix, and give its test a way to fail off-BSD #273

Description

@StefanSteiner

Summary

0.7.3 added one line to the health listener's accept path that fixes a real, pre-existing macOS/BSD bug — plausibly the release's largest resilience win. It has no changelog entry, its code comment does not name the reason it exists, and the test covering it passes trivially on the one platform CI would notice a revert on.

// hyperdb-mcp/src/daemon/health.rs:139-147
            match self.listener.accept() {
                Ok((stream, _addr)) => {
                    if let Err(error) = stream.set_nonblocking(false) {
                        warn!(
                            error = %error,
                            "could not make accepted health connection blocking"
                        );
                        continue;
                    }

The bug it fixes

HealthListener::bind puts the listener in non-blocking mode so the accept loop can poll for shutdown:

// hyperdb-mcp/src/daemon/health.rs:115-120
    pub fn bind(port: u16) -> std::io::Result<Self> {
        let addr = std::net::SocketAddr::from(([127, 0, 0, 1], port));
        let listener = TcpListener::bind(addr)?;
        listener.set_nonblocking(true)?;
        let port = listener.local_addr()?.port();
        Ok(Self { listener, port })
    }

On Linux, accept() returns a socket with default flags regardless of the listener's. On BSD-derived kernels — macOS included — the accepted socket inherits O_NONBLOCK from the listener. So on macOS the per-connection handler got a non-blocking socket, and its very first read_line returned WouldBlock:

// hyperdb-mcp/src/daemon/health.rs:193-196
    loop {
        line.clear();
        match reader.read_line(&mut line) {
            Ok(0) => break,
// hyperdb-mcp/src/daemon/health.rs:220
            Err(_) => break,

Err(_) => break does not distinguish WouldBlock from a real error, so the connection closed roughly 7 µs after being accepted — before the client had sent its first byte. Reproduced in a scratch binary during review.

Health liveness gates discovery (is_daemon_alive, hyperdb-mcp/src/daemon/discovery.rs:310), restart reporting, and heartbeats, so on macOS every one of those was racing a socket that hung up immediately. That makes this a bigger correctness win than most of what the release documents at length.

Three things make it fragile

No changelog entry. hyperdb-mcp/CHANGELOG.md has no mention of set_nonblocking, blocking accepts, WouldBlock, the listener, macOS, or BSD. The nearest entry, ### Fixed at hyperdb-mcp/CHANGELOG.md:232-235, covers health-port targeting and the engine mutex — not this. A user who hit the macOS symptom has nothing to match against.

The comment does not say why the line exists. All it records is the failure to apply it:

// hyperdb-mcp/src/daemon/health.rs:142-145
                        warn!(
                            error = %error,
                            "could not make accepted health connection blocking"
                        );

Nothing names BSD inheritance, macOS, or the WouldBlock-closes-the-connection consequence. Read cold, set_nonblocking(false) on a socket that is already blocking on Linux looks like a redundant defensive call — exactly the shape a future simplifier deletes with confidence.

The test cannot fail on Linux. health_listener_waits_for_command_after_accept (hyperdb-mcp/tests/daemon_tests.rs:237) is platform-agnostic and asserts that an accepted-but-idle connection survives long enough to send a delayed command. On Linux, accepted sockets are blocking whether or not the fix is present, so the test passes either way. Reverting the one line would keep the Linux leg of CI green; only the macOS leg would catch it — and see #271 for how much daemon coverage macOS is currently carrying.

There is also a stale comment nearby, left over from the accept-poll change in the same release:

// hyperdb-mcp/tests/daemon_tests.rs:317-319
            // Keep the already-accepted first socket idle for more than two
            // additional 100ms listener polls before sending its first command.
            std::thread::sleep(Duration::from_millis(350));

The listener poll is 5 ms now, not 100 ms (hyperdb-mcp/src/daemon/health.rs:160), so "two additional 100ms listener polls" no longer describes anything. The 350 ms sleep is still fine as an absolute duration; only the justification is wrong.

Fix direction

Three small, independent pieces:

  • A ### Fixed entry in hyperdb-mcp/CHANGELOG.md naming the platform and the symptom, so the macOS behavior is findable.
  • Expand the code comment to state the BSD O_NONBLOCK inheritance and what happens without the call. One sentence is enough to stop it reading as boilerplate.
  • Give the test something that can fail off-BSD. Options worth weighing: assert directly on the accepted socket's blocking state where the platform exposes it, or have the test set up the listener the way bind does and confirm the handler's first read blocks rather than returning immediately. Either way the goal is that deleting the line turns some leg of CI red on a platform that actually runs.

Distinguishing WouldBlock from a genuine error at hyperdb-mcp/src/daemon/health.rs:220 is worth considering in the same pass — the current Err(_) => break is what turned an inherited flag into a dropped connection, and it would do the same for any future source of WouldBlock.

Provenance

Retrospective adversarial review of release 0.7.3 (2026-08-28), now part of 1.0.0-rc.2. Verified against current main at 06c5da1.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions