From e71a4523afc014aac672a8e1c088aaa619ad09f8 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Sat, 25 Jul 2026 11:03:49 -0700 Subject: [PATCH] Close the socket after each response instead of leaking a descriptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NetworkSession is created per connection and nothing retains it: the only strong reference is the `[self]` capture inside the receive handler, which is released as soon as that handler returns. The send completion then cancelled the connection through `weak self`, so by the time it fired self was usually already nil and NWConnection.cancel() never ran. Every served request leaked one file descriptor. The process holds a 256 soft file limit, so a client polling GET /status once a second — which the bundled browser extension does — exhausts it in well under an hour. Observed in the wild: 2,532 sockets stuck in CLOSED state and the listener no longer accepting connections while the app appeared healthy. Capture the connection rather than the session, so cancellation is guaranteed regardless of session lifetime. The regression test counts the test process's own descriptors across a burst of requests. Against the previous code it reports growth of exactly 200 over 200 requests; with the fix, growth stays near zero and the listener still answers after the burst. Co-Authored-By: Claude Opus 5 --- .../VoxClawCore/Network/NetworkSession.swift | 12 +++-- .../NetworkListenerIntegrationTests.swift | 45 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/Sources/VoxClawCore/Network/NetworkSession.swift b/Sources/VoxClawCore/Network/NetworkSession.swift index 1d6b326..ca1b790 100644 --- a/Sources/VoxClawCore/Network/NetworkSession.swift +++ b/Sources/VoxClawCore/Network/NetworkSession.swift @@ -331,13 +331,17 @@ final class NetworkSession: Sendable { headers += "\r\n" var responseData = headers.data(using: .utf8) ?? Data() responseData.append(bodyData) - connection.send(content: responseData, completion: .contentProcessed { [weak self] _ in - self?.connection.cancel() + // Capture the connection, not self: nothing retains the session past + // the receive handler, so a `weak self` here is usually already nil by + // the time the send completes and the socket is never closed (leaking + // one fd per request until the listener stops accepting entirely). + connection.send(content: responseData, completion: .contentProcessed { [connection] _ in + connection.cancel() }) } else { headers += "\r\n" - connection.send(content: headers.data(using: .utf8), completion: .contentProcessed { [weak self] _ in - self?.connection.cancel() + connection.send(content: headers.data(using: .utf8), completion: .contentProcessed { [connection] _ in + connection.cancel() }) } } diff --git a/Tests/VoxClawCoreTests/NetworkListenerIntegrationTests.swift b/Tests/VoxClawCoreTests/NetworkListenerIntegrationTests.swift index cc883b7..2e33860 100644 --- a/Tests/VoxClawCoreTests/NetworkListenerIntegrationTests.swift +++ b/Tests/VoxClawCoreTests/NetworkListenerIntegrationTests.swift @@ -38,6 +38,51 @@ struct NetworkListenerIntegrationTests { #expect(!body.contains(".local")) } + /// Each served request must close its socket. The session that owns the + /// connection is not retained past its receive handler, so a response path + /// that cancels via `weak self` silently leaks one descriptor per request + /// and the listener eventually stops accepting entirely. + @Test func servedRequestsDoNotLeakDescriptors() async throws { + let appState = AppState() + let settings = SettingsManager() + let listener = NetworkListener(port: Self.testPort, serviceName: nil, appState: appState, settings: settings) + + try listener.start(onReadRequest: { _ in }) + defer { listener.stop() } + + try await waitForListener(port: Self.testPort) + + let url = URL(string: "http://localhost:\(Self.testPort)/status")! + // Don't let URLSession pool connections; we want each request's server-side + // socket to be the only thing that could accumulate. + let config = URLSessionConfiguration.ephemeral + config.httpShouldUsePipelining = false + let session = URLSession(configuration: config) + defer { session.invalidateAndCancel() } + + // Warm up so one-time allocations aren't counted as growth. + for _ in 0..<10 { _ = try await session.data(from: url) } + let before = Self.openDescriptorCount() + + let requestCount = 200 + for _ in 0.. Int { + (try? FileManager.default.contentsOfDirectory(atPath: "/dev/fd").count) ?? 0 + } + @Test func readEndpointAcceptsJSON() async throws { let appState = AppState() let settings = SettingsManager()