Skip to content

SocketPool: keep a descriptor's registration when a closed socket's is reused - #244

Open
lhoward wants to merge 1 commit into
swhitty:mainfrom
PADL:socketpool-stale-registration
Open

lhoward wants to merge 1 commit into
swhitty:mainfrom
PADL:socketpool-stale-registration

Conversation

@lhoward

@lhoward lhoward commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

When a socket is closed while it's suspended on a SocketPool, the next socket to get the same file descriptor can hang for good: its readiness is never reported. On Linux, the same bug surfaces as the intermittent epoll_ctl EPOLL_CTL_MOD / ENOENT failures in CI, for example server_StartsOnIP4Socket and server_AllowsExistingConnectionsToDisconnect_WhenStopped.

Cause

kQueue and ePoll each keep a record, existing, of the events each descriptor is registered for, and they trust it. Closing a socket makes the kernel drop its registration, but the record keeps it:

  1. The record goes stale. Cancelling the closed socket's suspension removes its events, and that removal fails because the descriptor has already left the kqueue or epoll set. removeEvents threw before it updated existing, so the stale entry stayed.

  2. The reused descriptor is never registered. A socket later given the same descriptor was taken to be registered already:

    • kQueue.addEvents skipped the EV_ADD.
    • ePoll.setEvents either returned early because the recorded events matched, or used EPOLL_CTL_MOD, which fails with ENOENT.

    Either way, the new socket's suspension was never woken.

The same happens when the descriptor is reused while the old socket's suspension is still pending.

Changes

  • kQueue
    • removeEvents updates existing before removing each event, so a failed removal can't leave a stale entry. Removals still throw on failure, as the existing tests expect.
    • addEvents always issues EV_ADD. It's idempotent for a filter the kqueue already has.
  • ePoll
    • addEvents always tells the kernel.
    • setEvents updates existing before the change.
    • It falls back from EPOLL_CTL_MOD to EPOLL_CTL_ADD on ENOENT, and from ADD to MOD on EEXIST.
    • A failed change leaves the descriptor recorded as unregistered.

The cost is a kevent or epoll_ctl call for an event already registered, when a second waiter suspends on the same descriptor and event.

Tests

Two new SocketPoolTests use the platform's real pool: kQueue on Darwin, ePoll on Linux. Each suspends a socket, then dup2s a new socket over its descriptor. That closes the old socket and hands its descriptor to the new one in a single step, so no other test can take the descriptor in between. Each test then makes the new socket readable and expects its suspension to be woken within 1 s:

  • reusedDescriptor_IsWoken_AfterTheSuspendedSocketIsReplacedAndCancelled: the old suspension is cancelled before the new socket suspends.
  • reusedDescriptor_IsWoken_WhileTheReplacedSocketIsStillSuspended: the new socket suspends while the old suspension is still pending.

On macOS (Swift 6.3.3) both fail without this change, by timing out, and pass with it. The full suite passes: 474 tests. I couldn't build the ePoll change on macOS, so Linux CI is its first build and test. The Linux jobs may also show the unrelated sendMessage_WithPacketInfo_RoundTripsDatagram failure that #243 fixes.

Remaining hazard, not addressed here

AsyncSocket.close() closes the descriptor without telling the pool. So a suspension left pending on a closed socket can be woken by whichever socket reuses the descriptor. Having close() resume its own suspensions with an error first would need a new pool API, so it's better proposed separately.

We found this in SwiftOCA, whose device endpoints run a SocketPool per endpoint and close controller sockets while their reads are suspended.

🤖 Generated with Claude Code

https://claude.ai/code/session_012Fq7ie1LzJERrh9uRuZZWE

…s reused

The kQueue and ePoll queues record which events each descriptor is
registered for, and trust that record. When a socket is closed while it
is suspended, the kernel drops its registration, but the record keeps it:

- Removing its events afterwards fails, because the descriptor has
  already left the kqueue or epoll set. That failure was thrown before
  the record was updated, so the entry stayed.
- A socket later given the same descriptor was taken to be registered
  already. kQueue skipped the EV_ADD. ePoll either skipped the change or
  used EPOLL_CTL_MOD, which fails with ENOENT. Either way the new socket
  was never woken.

Update the record before removing events. When adding events, always
tell the kernel: kqueue's EV_ADD just updates a filter it already has.
For epoll, fall back from EPOLL_CTL_MOD to EPOLL_CTL_ADD on ENOENT and
from ADD to MOD on EEXIST. A failed change leaves the descriptor recorded
as unregistered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Fq7ie1LzJERrh9uRuZZWE
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.72%. Comparing base (bd32d43) to head (f38b812).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #244   +/-   ##
=======================================
  Coverage   93.72%   93.72%           
=======================================
  Files          72       72           
  Lines        3777     3779    +2     
=======================================
+ Hits         3540     3542    +2     
  Misses        237      237           

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

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