[Bug]: connectionsBeyondTheCapAreClosed is flaky - it opens 128 connections against a listen backlog of 16

[FRO777]

Commit

fd3c0b8 (Add optional Gemma 4 image support (#144)). Both constants involved are unchanged since e65da9e.

Mac hardware and memory

Apple M3 Pro, 18 GB

macOS and Swift versions

macOS 27.0 (26A5416b), Swift 6.4 (swiftlang-6.4.0.27.1)

Does this require the installed model?

No.

Reproduction steps

for i in $(seq 1 20); do Scripts/test.sh --filter connectionsBeyondTheCapAreClosed; done

Expected behavior

The test passes on every run.

Observed behavior

It fails 17 of 20 runs on this machine:

✘ Test connectionsBeyondTheCapAreClosed() recorded an issue at
  ServerIngressHardeningTests.swift:301:6: Caught error: .systemCall("connect", 54)

54 is ECONNRESET, thrown by connectedSocket (Tests/TurboFieldfareServer/HTTPServerTests.swift:907) when the blocking Darwin.connect fails.

It passes on GitHub Actions, so it shows up as a test that is green in CI and mostly red locally rather than as an obvious break.

Cause

Two values disagree:

• connectionsBeyondTheCapAreClosed opens TurboFieldfareHTTPServer.maximumConnections = 128 sockets in a tight synchronous loop, before it waits for the accept loop at all.
• TurboFieldfareHTTPServer.start sets .serverChannelOption(ChannelOptions.backlog, value: 16) (Sources/TurboFieldfareServer/Core/HTTPServer.swift:59).

So the test can need up to 128 connections sitting in the listen queue while NIO's accept loop is still draining it, but the queue holds 16. Once it is full, Darwin answers further SYNs with RST rather than queueing them, and the blocking connect() returns ECONNRESET. Whether the accept loop stays ahead of the test's loop is a race, which is why the failure is intermittent rather than constant.

Confirming experiment

Changing only that one line to value: 256 and re-running the same 20 iterations:

ChannelOptions.backlog	passes
16 (current)	3 / 20
256	20 / 20

Nothing else was changed, and the line was reverted afterwards.

Possible directions

Not sending a patch, because the right fix depends on what the backlog of 16 is for:

1. If 16 is deliberate, the test is asserting something the server does not promise. It could pace its connects, or tolerate and retry ECONNRESET while filling up to the cap.
2. If 16 is incidental, it looks low for a server whose own connection cap is 128 — a real client burst would meet the same RST. kern.ipc.somaxconn is 128 on this machine, so a backlog of 128 would be accepted as requested.

I have not measured whether a backlog of 16 causes trouble for real clients. The only thing established here is that it is what makes this test unstable.

[FRO777]

Following up with a fix I have run, in case it saves you the work.

I went with the test side rather than the backlog. A test that needs 128 connections queued at once is asserting something start never promised, and widening the backlog to satisfy it would change real client behaviour to quiet a test. This keeps the assertion the test was written for — that connection 129 is refused — and lets the accept loop drain as it fills, holding at most 8 outstanding, comfortably inside the 16:

         let cap = TurboFieldfareHTTPServer.maximumConnections
         var sockets: [Int32] = []
         defer { sockets.forEach { _ = Darwin.close($0) } }
-        for _ in 0..<cap {
+        // The listen backlog is 16 (HTTPServer.start), so the kernel starts
+        // answering with RST — connect() fails ECONNRESET — as soon as the accept
+        // loop falls that far behind. Opening all 128 in one burst raced that
+        // queue, so keep the outstanding count well inside the backlog while
+        // filling up to the cap.
+        let backlogMargin = 8
+        for index in 0..<cap {
             sockets.append(try connectedSocket(port: port))
+            let queueDeadline = Date().addingTimeInterval(4)
+            while await server.acceptedConnectionCount < index + 1 - backlogMargin,
+                  Date() < queueDeadline {
+                try await Task.sleep(nanoseconds: 1_000_000)
+            }
         }
         // The accept loop runs on the event loop; wait for it to catch up.

Same 20 iterations as in the report above:

	passes
unchanged	3 / 20
backlog raised to 256	20 / 20
this patch, backlog left at 16	20 / 20

Adds roughly 0.2 s to the test on this machine.

This deliberately does not answer whether a backlog of 16 is right for a server whose own cap is 128 — that is the more interesting question in the original report, and it needs a measurement I have not made. If you would rather settle that first and fix the test as a consequence, this patch is the wrong one to take.

Happy to open it as a PR if that is easier than applying it by hand.

[drumih]

@FRO777 #153 pls take a look
and thank you very much for the report!