mux web refuses a port another hub already listens on

7d4cf053 fix/webhub-refuse-taken-port → main

3 files changed

src/cli/webhub_main.zig

Old		New
@@ -143,10 +143,24 @@ pub fn main(args: []const [:0]const u8) !u8 {
143	var hub = try webhub.Hub.init(arena, specs);	143	var hub = try webhub.Hub.init(arena, specs);
144	defer hub.deinit();	144	defer hub.deinit();
145		145
146	const addr = std.net.Address.parseIp("127.0.0.1", parsed.port) catch unreachable;	146	var listener = webhub.listenLocal(parsed.port) catch \|err\| switch (err) {
147	var listener = addr.listen(.{ .reuse_address = true }) catch \|err\| {	147	// `mux d`'s refusal, in the hub's own words: a port another hub owns
148	std.debug.print("mux web: cannot bind 127.0.0.1:{d}: {s}\n", .{ parsed.port, @errorName(err) });	148	// is not this hub's to take, and sharing it strands both — they each
149	return 1;	149	// keep serving a wall and a browser reaches whichever the kernel
		150	// hands the connection to. Same one-liner and same rc as
		151	// `mux d: a daemon is already running on PATH`, and knowingly the
		152	// same wrong-ish message when the port belongs to some other
		153	// program entirely: the advice is right either way, which is the
		154	// trade decisions.md already records for the daemon's own
		155	// AddressInUse.
		156	error.AddressInUse => {
		157	std.debug.print("mux web: a hub is already running on 127.0.0.1:{d} (--port N serves elsewhere)\n", .{parsed.port});
		158	return 1;
		159	},
		160	else => {
		161	std.debug.print("mux web: cannot bind 127.0.0.1:{d}: {s}\n", .{ parsed.port, @errorName(err) });
		162	return 1;
		163	},
150	};	164	};
151	defer listener.deinit();	165	defer listener.deinit();
152		166

src/client/webhub.zig

@@ @@ -12,6 +12,52 @@ const client = @import("client"); @@
+pub const default_port: u16 = 7681;
+/// Take 127.0.0.1:`port` for this hub, or hand back the kernel's refusal. A
+/// port is to a hub what the socket path is to a daemon, so the rule is
+/// `sockpath.claim`'s: never share an address a live peer already answers on.
+/// Spelled here rather than left to `std.net.Address.listen` because that
+/// function's one `reuse_address` bit sets SO_REUSEPORT alongside SO_REUSEADDR
+/// for every family but unix, and SO_REUSEPORT is not the permissive-restart
+/// flag its name suggests: it lets any number of processes bind one address
+/// and port at once and has the kernel spread incoming connections across all
+/// of them. Two `mux web` runs on 7681 therefore both bound, `ss` showed two
+/// LISTEN rows, and a browser reached whichever the kernel picked — half the
+/// tabs got a stale hub's wall, with nothing on screen saying so.
+///
+/// SO_REUSEADDR alone stays on, for the one job it actually does here: a hub
+/// restarted while the connections its predecessor accepted are still in
+/// TIME_WAIT gets its port back instead of a spurious refusal. A port a LIVE
+/// listener holds is `error.AddressInUse` with or without it — measured, not
+/// assumed — and that error is what `mux web` refuses on.
+///
+/// Port 0 asks the kernel for a free port. `mux web` rejects it at parse,
+/// because the announced address would not be the bound one; the tests below
+/// use it to take a port nothing else on the box owns.
+pub fn listenLocal(port: u16) !std.net.Server {
+    const addr = std.net.Address.parseIp("127.0.0.1", port) catch unreachable;
+    const fd = try std.posix.socket(
+        addr.any.family,
+        std.posix.SOCK.STREAM | std.posix.SOCK.CLOEXEC,
+        std.posix.IPPROTO.TCP,
+    );
+    var server: std.net.Server = .{ .listen_address = undefined, .stream = .{ .handle = fd } };
+    errdefer server.stream.close();
+    try std.posix.setsockopt(
+        fd,
+        std.posix.SOL.SOCKET,
+        std.posix.SO.REUSEADDR,
+        &std.mem.toBytes(@as(c_int, 1)),
+    );
+    var len = addr.getOsSockLen();
+    try std.posix.bind(fd, &addr.any, len);
+    try std.posix.listen(fd, 128);
+    // The address as BOUND, read back from the kernel: with port 0 it is not
+    // the one asked for, and `listen_address` is where a caller reads the
+    // port it actually got.
+    try std.posix.getsockname(fd, &server.listen_address.any, &len);
+    return server;
+}
+/// ONE number bounds two things, a property of std.http.Server: the connection
+/// Reader's buffer is both the max HTTP header size and the max inbound
+/// WebSocket message. 64 KiB — the browser chunks pastes at 32 KiB, so nothing
+test "listenLocal: a port a live hub holds is refused, and its flags are REUSEADDR without REUSEPORT" {
+    // Port 0 for the first bind, so the port under test is one the kernel
+    // just said was free rather than a number this file hopes nothing on the
+    // box is using. The second bind then asks for that exact port: two hubs,
+    // one `--port`, which is the bug this refusal exists for.
+    const port = port: {
+        var first = try listenLocal(0);
+        defer first.deinit();
+        const p = first.listen_address.getPort();
+        try std.testing.expect(p != 0);
+        try std.testing.expectError(error.AddressInUse, listenLocal(p));
+        // Asked of the DESCRIPTOR, not read off the call that made it — the
+        // two flags are the whole of this fix, and the call that used to set
+        // them set both from one `reuse_address = true` (serve.zig's cloexec
+        // test is the same shape, for the same reason: a default going
+        // quietly wrong is invisible at the call site).
+        var v: c_int = undefined;
+        try std.posix.getsockopt(
+            first.stream.handle,
+            std.posix.SOL.SOCKET,
+            std.posix.SO.REUSEADDR,
+            std.mem.asBytes(&v),
+        );
+        try std.testing.expect(v != 0);
+        try std.posix.getsockopt(
+            first.stream.handle,
+            std.posix.SOL.SOCKET,
+            std.posix.SO.REUSEPORT,
+            std.mem.asBytes(&v),
+        );
+        try std.testing.expectEqual(@as(c_int, 0), v);
+        break :port p;
+    };
+    // The hub that held it is gone, so the port is takeable again: the
+    // refusal is about a LIVE listener, not about the number having been
+    // used once. Without SO_REUSEADDR this is where a restart would start
+    // failing as soon as a browser had ever connected.
+    var second = try listenLocal(port);
+    defer second.deinit();
+    try std.testing.expectEqual(port, second.listen_address.getPort());
+}
+test "ws path by id: parses, no range opinion" {
+    try std.testing.expectEqual(@as(?u32, 0), wsTileId("/ws/0"));
+    try std.testing.expectEqual(@as(?u32, 41), wsTileId("/ws/41"));

test/e2e_06_web.sh

@@ @@ -496,6 +496,40 @@ sleep 3 @@
+    echo "e2e FAIL: host wall: B's tile did not heal onto its own id $DWID_B0:"
+    curl -s "$DWORIG/tiles"; exit 1; }
+# A port a live hub owns is refused, in `mux d`'s words. Asked of a REAL
+# second process while the first is serving, because the bug was one the
+# kernel allowed: SO_REUSEPORT let both bind, `ss` showed two LISTEN rows,
+# and a browser reached whichever the kernel handed the connection to.
+set +e
+# Under timeout, like the QUIC edition in e2e_01_boot.sh and for the same
+# reason: a second hub that WINS the port serves until killed, so the
+# regression has to show up as 124 rather than as a hung suite.
+XDG_STATE_HOME="$DWSTATE" timeout 10 "$MUX" web --port "$WPORT4" > "$OUT.dwtaken" 2>&1
+RC=$?
+set -e
+[ "$RC" -eq 1 ] || {
+    echo "e2e FAIL: host wall: a second hub on $WPORT4 exited $RC (want 1; 124 means it bound and served)"
+    cat "$OUT.dwtaken"; exit 1; }
+grep -q "already running" "$OUT.dwtaken" || {
+    echo "e2e FAIL: host wall: the taken-port refusal did not say already running:"
+    cat "$OUT.dwtaken"; exit 1; }
+grep -q "serving" "$OUT.dwtaken" && {
+    echo "e2e FAIL: host wall: the second hub announced itself on a taken port:"
+    cat "$OUT.dwtaken"; exit 1; }
+# Asked of the OS, not of either hub: exactly one socket LISTENs there.
+# /proc/net/tcp rather than ss or lsof, for e2e_01_boot.sh's reason — always
+# present, no privileges — with 0A the LISTEN state and 0100007F the
+# little-endian hex the file spells 127.0.0.1 in.
+WHEX=$(printf '0100007F:%04X' "$WPORT4")
+LISTENERS=$(awk -v a="$WHEX" '$2 == a && $4 == "0A" { n++ } END { print n+0 }' /proc/net/tcp)
+[ "$LISTENERS" -eq 1 ] || {
+    echo "e2e FAIL: host wall: $LISTENERS listeners on $WPORT4, want 1:"
+    grep -i "$WHEX" /proc/net/tcp || true; exit 1; }
+# ...and the survivor is the first hub, still serving the wall it built:
+# the refusal happens in the newcomer, and the running hub never hears it.
+[ -n "$(curl -s "$DWORIG/tiles")" ] || {
+    echo "e2e FAIL: host wall: the refused hub disturbed the live one"; exit 1; }
+softkill "$W4PID" || true
+wait_pid_gone "$W4PID" "host wall: hub killed by tracked pid"
+W4PID=""