Skip to content

Fix socket fd leaks on AsyncSocket connect/accept failure paths - #227

Merged
swhitty merged 3 commits into
swhitty:mainfrom
ianegordon:ian/tvt-1077-asyncsocket-leaks-the-socket-fd-on-connectaccept-failure
Jul 22, 2026
Merged

Fix socket fd leaks on AsyncSocket connect/accept failure paths#227
swhitty merged 3 commits into
swhitty:mainfrom
ianegordon:ian/tvt-1077-asyncsocket-leaks-the-socket-fd-on-connectaccept-failure

Conversation

@ianegordon

Copy link
Copy Markdown
Contributor

Summary

Two file-descriptor leaks in AsyncSocket:

  • connected(to:pool:timeout:) — the freshly created Socket was never closed when AsyncSocket.init threw, connect(to:) failed terminally (e.g. ECONNREFUSED), or the timeout fired. Every failed or timed-out client connect leaked one fd. Since withThrowingTimeout awaits the body task's result after cancelling it, a catch inside the body is guaranteed to run on the timeout path too.
  • accept() — if AsyncSocket.init threw after socket.accept() returned a valid descriptor, the accepted fd was orphaned.

Both sites now close the underlying socket before rethrowing the original error (try? close() so the original error is preserved).

Testing

  • New connected_ThrowsError_WhenConnectFails connects to a nonexistent unix-socket path — deterministic (synchronous failure, no network timing) — and code coverage confirms it executes the new close-on-failure path.
  • The accept() catch path isn't reachable in unit form (would require forcing fcntl F_SETFL to fail on a valid fd); it mirrors the tested connect path.
  • Full suite passes: 450 tests in 51 suites.

🤖 Generated with Claude Code

connected(to:pool:timeout:) never closed the freshly created socket
when AsyncSocket.init, connect, or the timeout failed; accept() leaked
the accepted descriptor if AsyncSocket.init threw. Both now close the
underlying socket before rethrowing the original error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.85%. Comparing base (4e246d3) to head (6508863).

Files with missing lines Patch % Lines
FlyingSocks/Sources/AsyncSocket.swift 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #227      +/-   ##
==========================================
- Coverage   92.88%   92.85%   -0.04%     
==========================================
  Files          71       71              
  Lines        3727     3735       +8     
==========================================
+ Hits         3462     3468       +6     
- Misses        265      267       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

ianegordon and others added 2 commits July 20, 2026 22:51
Counts open descriptors around 50 failing connects; fails against the
pre-fix code (+50 fds) and passes with the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Counting process-wide descriptors is nondeterministic when the full
suite runs in parallel: CI failed with +86 (macOS) and +54 (Linux) of
unrelated churn in the measurement window. Deterministic coverage of
the cleanup path is tracked separately via an ownership-transfer
helper refactor.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@swhitty
swhitty merged commit db8cbe7 into swhitty:main Jul 22, 2026
11 of 13 checks passed
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.

2 participants