From 5a51e2d6b9aa8b492ca79999cc9c545c2f52f6cf Mon Sep 17 00:00:00 2001 From: m5r Date: Wed, 21 Oct 2026 21:32:40 +0200 Subject: [PATCH] tests: a failed server check fails the test instead of hanging it, and the tls idle tests leave the handshake room --- src/server/doh_server.zig | 8 ++- src/server/dot_server.zig | 76 ++++++++++++---------- src/server/resolver_integration_test.zig | 11 ++-- src/server/tcp_server_integration_test.zig | 46 +++++++------ src/server/udp_server_integration_test.zig | 35 +++++----- src/web/server_integration_test.zig | 4 +- 6 files changed, 104 insertions(+), 76 deletions(-) diff --git a/src/server/doh_server.zig b/src/server/doh_server.zig index 753ac44..07f5182 100644 --- a/src/server/doh_server.zig +++ b/src/server/doh_server.zig @@ -1216,9 +1216,11 @@ fn twoPostsOneConnection(io: std.Io, remote: net.IpAddress) anyerror!void { try conn.end(); } -/// Long enough for a loopback round trip, short enough that the `bounded` -/// budget still fails a server that never reclaims the connection. -const short_idle: std.Io.Clock.Duration = .{ .raw = .fromMilliseconds(300), .clock = .awake }; +/// The TLS handshake runs inside this budget too: a Debug-build handshake takes +/// about 100 ms on an idle machine, and a loaded CI runner failed it at +/// 300 ms. The `bounded` budget still fails a server that never reclaims the +/// connection. +const short_idle: std.Io.Clock.Duration = .{ .raw = .fromSeconds(2), .clock = .awake }; /// One request, then silence on a connection the client keeps open. The server /// owes no answer, so the only thing that can end the read is the idle budget. diff --git a/src/server/dot_server.zig b/src/server/dot_server.zig index 1ad6318..07224e8 100644 --- a/src/server/dot_server.zig +++ b/src/server/dot_server.zig @@ -326,9 +326,11 @@ fn bareHandler(up: *upstream_owner.Owner) handler.Handler { const test_budget: std.Io.Clock.Duration = .{ .raw = .fromSeconds(5), .clock = .awake }; -/// Short enough to keep the idle-timeout test quick, long enough that a -/// loopback handshake cannot lose to scheduling and time out on its own. -const short_idle: std.Io.Clock.Duration = .{ .raw = .fromMilliseconds(300), .clock = .awake }; +/// The handshake runs inside this budget too: a Debug-build handshake takes +/// about 100 ms on an idle machine, and a loaded CI runner failed it at +/// 300 ms. `test_budget` still fails a server that never reclaims the +/// connection. +const short_idle: std.Io.Clock.Duration = .{ .raw = .fromSeconds(2), .clock = .awake }; /// What the store advertises in production; std.crypto.tls.Client cannot send /// ALPN in 0.16, so this also proves no-ALPN clients still connect. @@ -580,6 +582,14 @@ test "dot: two framed queries share one TLS connection" { var group: std.Io.Group = .init; try group.concurrent(io, DotServer.serve, .{ &server, io }); + // Deferred so a failed check still stops the server: a `serve` left in + // accept makes `threaded.deinit` wait for it forever. + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, dotKeepAlive, .{ io, server_address }); @@ -591,11 +601,6 @@ test "dot: two framed queries share one TLS connection" { try testing.expectEqual(@as(u64, 0), stats.idle_timeouts); try testing.expectEqual(@as(u64, 0), stats.connection_errors); try testing.expectEqual(@as(u64, 2), h.stats.queries.load(.monotonic)); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "dot: a transport EOF without close_notify is a connection error, not a crash" { @@ -621,6 +626,12 @@ test "dot: a transport EOF without close_notify is a connection error, not a cra var group: std.Io.Group = .init; try group.concurrent(io, DotServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, dotDropWithoutCloseNotify, .{ io, server_address }); try waitForCounter(io, &server.core.stats.connection_errors, 1); @@ -630,11 +641,6 @@ test "dot: a transport EOF without close_notify is a connection error, not a cra try testing.expectEqual(@as(u64, 0), stats.tls_handshake_failures); try testing.expectEqual(@as(u64, 1), stats.connection_errors); try testing.expectEqual(@as(u64, 1), h.stats.queries.load(.monotonic)); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "dot: plain TCP bytes fail the handshake and are counted" { @@ -660,6 +666,12 @@ test "dot: plain TCP bytes fail the handshake and are counted" { var group: std.Io.Group = .init; try group.concurrent(io, DotServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, dotGarbageHandshake, .{ io, server_address }); try waitForCounter(io, &server.stats.tls_handshake_failures, 1); @@ -669,11 +681,6 @@ test "dot: plain TCP bytes fail the handshake and are counted" { try testing.expectEqual(@as(u64, 1), stats.tls_handshake_failures); try testing.expectEqual(@as(u64, 0), stats.connection_errors); try testing.expectEqual(@as(u64, 0), h.stats.queries.load(.monotonic)); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } /// Ruling 6 across a live listener: a reload retires the old generation @@ -747,6 +754,12 @@ test "dot: a reload serves new handshakes without breaking the old connection" { var group: std.Io.Group = .init; try group.concurrent(io, DotServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, dotReloadKeepsOldConnAndServesNew, .{ io, server_address, &env }); @@ -760,11 +773,6 @@ test "dot: a reload serves new handshakes without breaking the old connection" { const store_stats = env.store.snapshotStats(); try testing.expectEqual(@as(u64, 1), store_stats.reloads); try testing.expectEqual(@as(u64, 0), store_stats.reload_failures); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } /// S3.4 across a live listener: a cert PATH change — new files, not rewritten @@ -838,6 +846,12 @@ test "dot: a cert path change serves the new certificate on the next handshake" var group: std.Io.Group = .init; try group.concurrent(io, DotServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, dotPathChangeServesNewCert, .{ io, server_address, &env }); @@ -849,11 +863,6 @@ test "dot: a cert path change serves the new certificate on the next handshake" const store_stats = env.store.snapshotStats(); try testing.expectEqual(@as(u64, 1), store_stats.reloads); try testing.expectEqual(@as(u64, 0), store_stats.reload_failures); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "dot: an idle connection is closed with close_notify and counted" { @@ -882,6 +891,12 @@ test "dot: an idle connection is closed with close_notify and counted" { var group: std.Io.Group = .init; try group.concurrent(io, DotServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, dotIdleUntilServerCloses, .{ io, server_address }); @@ -890,9 +905,4 @@ test "dot: an idle connection is closed with close_notify and counted" { try testing.expectEqual(@as(u64, 1), stats.idle_timeouts); try testing.expectEqual(@as(u64, 0), stats.tls_handshake_failures); try testing.expectEqual(@as(u64, 0), stats.connection_errors); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } diff --git a/src/server/resolver_integration_test.zig b/src/server/resolver_integration_test.zig index 5e6403c..8125140 100644 --- a/src/server/resolver_integration_test.zig +++ b/src/server/resolver_integration_test.zig @@ -290,6 +290,13 @@ test "the whole resolver answers over udp and tcp and fails over to a healthy up var group: std.Io.Group = .init; try group.concurrent(io, udp_server.UdpServer.serve, .{ &udp, io }); try group.concurrent(io, tcp_server.TcpServer.serve, .{ &tcp, io }); + // Deferred so a failed check still stops both servers: a `serve` left in + // accept makes `threaded.deinit` wait for it forever. + defer { + udp.deinit(gpa, io); + tcp.deinit(io); + group.cancel(io); + } const client_address: net.IpAddress = try .parse("127.0.0.1", 0); var client = try client_address.bind(io, .{ .mode = .dgram }); @@ -335,8 +342,4 @@ test "the whole resolver answers over udp and tcp and fails over to a healthy up } try testing.expectEqual(bad_calls, bad.calls.load(.monotonic)); try testing.expectEqual(@as(u64, 3), good.calls.load(.monotonic)); - - udp.deinit(gpa, io); - tcp.deinit(io); - group.cancel(io); } diff --git a/src/server/tcp_server_integration_test.zig b/src/server/tcp_server_integration_test.zig index d7dc581..677d369 100644 --- a/src/server/tcp_server_integration_test.zig +++ b/src/server/tcp_server_integration_test.zig @@ -184,17 +184,20 @@ test "two length-prefixed queries share one connection" { var group: std.Io.Group = .init; try group.concurrent(io, tcp_server.TcpServer.serve, .{ &server, io }); + // Deferred so a failed check still stops the server: a `serve` left in + // accept makes `threaded.deinit` wait for it forever. + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, twoQueriesOnOneConnection, .{ io, server_address }); try testing.expectEqual(@as(u64, 1), server.core.stats.connections.load(.monotonic)); try testing.expectEqual(@as(u64, 0), server.core.stats.rejected_at_capacity.load(.monotonic)); try testing.expectEqual(@as(u64, 2), h.stats.queries.load(.monotonic)); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "the claimed slot records the connecting client" { @@ -215,6 +218,12 @@ test "the claimed slot records the connecting client" { var group: std.Io.Group = .init; try group.concurrent(io, tcp_server.TcpServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, twoQueriesOnOneConnection, .{ io, server_address }); @@ -226,11 +235,6 @@ test "the claimed slot records the connecting client" { try testing.expectEqual(net.IpAddress.ip4, std.meta.activeTag(peer)); try testing.expectEqualSlices(u8, &[_]u8{ 127, 0, 0, 1 }, &peer.ip4.bytes); try testing.expect(peer.ip4.port != 0); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } /// `std.Io.Group` takes only `Cancelable!void`, so the result is reported out @@ -405,17 +409,18 @@ test "an idle connection is closed and counted" { var group: std.Io.Group = .init; try group.concurrent(io, tcp_server.TcpServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, waitForServerClose, .{ io, server_address }); try testing.expectEqual(@as(u64, 1), server.core.stats.connections.load(.monotonic)); try testing.expectEqual(@as(u64, 1), server.core.stats.idle_timeouts.load(.monotonic)); try testing.expectEqual(@as(u64, 0), server.core.stats.connection_errors.load(.monotonic)); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "a zero-length message is a connection error" { @@ -439,16 +444,17 @@ test "a zero-length message is a connection error" { var group: std.Io.Group = .init; try group.concurrent(io, tcp_server.TcpServer.serve, .{ &server, io }); + defer { + server.deinit(io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } try bounded(io, sendZeroLength, .{ io, server_address }); try testing.expectEqual(@as(u64, 1), server.core.stats.connection_errors.load(.monotonic)); try testing.expectEqual(@as(u64, 0), server.core.stats.idle_timeouts.load(.monotonic)); - - server.deinit(io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } /// A prefix of 0 announces a message RFC 1035 §4.2.2 gives no meaning to, so diff --git a/src/server/udp_server_integration_test.zig b/src/server/udp_server_integration_test.zig index d09241c..2f9d0c0 100644 --- a/src/server/udp_server_integration_test.zig +++ b/src/server/udp_server_integration_test.zig @@ -121,6 +121,14 @@ test "a udp query is answered on the loopback" { var group: std.Io.Group = .init; try group.concurrent(io, udp_server.UdpServer.serve, .{ &server, io }); + // Deferred so a failed check still stops the server: a `serve` left in + // accept makes `threaded.deinit` wait for it forever. + defer { + server.deinit(gpa, io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } const client_address: net.IpAddress = try .parse("127.0.0.1", 0); var client = try client_address.bind(io, .{ .mode = .dgram }); @@ -135,11 +143,6 @@ test "a udp query is answered on the loopback" { try testing.expectEqual(@as(u64, 1), server.stats.received.load(.monotonic)); try testing.expectEqual(@as(u64, 0), server.stats.send_errors.load(.monotonic)); try testing.expectEqual(@as(u64, 1), h.stats.queries.load(.monotonic)); - - server.deinit(gpa, io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "a runt datagram is dropped and no reply is sent" { @@ -160,6 +163,12 @@ test "a runt datagram is dropped and no reply is sent" { var group: std.Io.Group = .init; try group.concurrent(io, udp_server.UdpServer.serve, .{ &server, io }); + defer { + server.deinit(gpa, io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } const client_address: net.IpAddress = try .parse("127.0.0.1", 0); var client = try client_address.bind(io, .{ .mode = .dgram }); @@ -171,11 +180,6 @@ test "a runt datagram is dropped and no reply is sent" { try testing.expectError(error.Timeout, client.receiveTimeout(io, &buf, silence)); try testing.expectEqual(@as(u64, 1), server.stats.received.load(.monotonic)); try testing.expectEqual(@as(u64, 1), h.stats.dropped_malformed.load(.monotonic)); - - server.deinit(gpa, io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "an oversize datagram arrives truncated and is dropped" { @@ -196,6 +200,12 @@ test "an oversize datagram arrives truncated and is dropped" { var group: std.Io.Group = .init; try group.concurrent(io, udp_server.UdpServer.serve, .{ &server, io }); + defer { + server.deinit(gpa, io); + group.await(io) catch |err| switch (err) { + error.Canceled => unreachable, + }; + } const client_address: net.IpAddress = try .parse("127.0.0.1", 0); var client = try client_address.bind(io, .{ .mode = .dgram }); @@ -213,11 +223,6 @@ test "an oversize datagram arrives truncated and is dropped" { try testing.expectError(error.Timeout, client.receiveTimeout(io, &buf, silence)); try testing.expectEqual(@as(u64, 1), server.stats.dropped_oversize.load(.monotonic)); try testing.expectEqual(@as(u64, 0), h.stats.queries.load(.monotonic)); - - server.deinit(gpa, io); - group.await(io) catch |err| switch (err) { - error.Canceled => unreachable, - }; } test "deinit ends a serve loop that is blocked on receive" { diff --git a/src/web/server_integration_test.zig b/src/web/server_integration_test.zig index 8a86cf3..e1cb229 100644 --- a/src/web/server_integration_test.zig +++ b/src/web/server_integration_test.zig @@ -644,7 +644,6 @@ test "cancellation returns promptly with an idle keep-alive connection open" { opened.wait(io) catch |err| switch (err) { error.Canceled => unreachable, }; - try testing.expect(!failed); // The listener task is canceled with a client parked in `receiveHead`. If // the cancel path awaited the connection group instead of canceling it, @@ -657,5 +656,8 @@ test "cancellation returns promptly with an idle keep-alive connection open" { client.cancel(io); web.deinit(io); + // Checked after teardown: a failed check before it would leave `serve` + // running, and `threaded.deinit` would wait for it forever. + try testing.expect(!failed); try testing.expect(elapsed.toMilliseconds() < settle.raw.toMilliseconds()); }