Skip to content

Close the socket after each response instead of leaking a descriptor - #12

Merged
malpern merged 1 commit into
mainfrom
fix-listener-socket-leak
Jul 25, 2026
Merged

Close the socket after each response instead of leaking a descriptor#12
malpern merged 1 commit into
mainfrom
fix-listener-socket-leak

Conversation

@malpern

@malpern malpern commented Jul 25, 2026

Copy link
Copy Markdown
Owner

The bug

NetworkSession is created per connection, and nothing retains itNetworkListener makes it a local, calls start(), and lets it go. The only strong reference is the [self] capture inside the receive handler, which is released as soon as that handler returns.

The response path then cancelled the connection through weak self:

connection.send(content: responseData, completion: .contentProcessed { [weak self] _ in
    self?.connection.cancel()   // self is usually already nil here
})

So NWConnection.cancel() typically never ran, and every served request leaked one file descriptor.

Why it matters

The app runs against a 256 soft file limit. A client polling GET /status once a second — which the bundled browser extension does — exhausts that in well under an hour.

Observed on a live instance: 2,532 sockets stuck in (CLOSED), 2,625 descriptors held, and the listener silently refusing new connections while the menu bar app looked perfectly healthy. Restarting the app "fixes" it until the count climbs again, which makes it easy to misread as a network or firewall problem.

The fix

Capture the connection instead of the session, so cancellation can't depend on session lifetime:

completion: .contentProcessed { [connection] _ in connection.cancel() }

Verification

The new test counts the test process's own open descriptors (via /dev/fd) across a burst of requests, then confirms the listener still answers afterward.

  • Against the previous code it fails with "descriptor count grew by 200 over 200 requests" — a clean 1:1 leak.
  • With the fix, growth stays near zero and the post-burst request returns 200.
  • Full suite: 280 tests in 35 suites pass.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@malpern
malpern merged commit e98004d into main Jul 25, 2026
2 checks passed
@malpern
malpern deleted the fix-listener-socket-leak branch July 25, 2026 18:46
@malpern malpern mentioned this pull request Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant