fix cancel/accept race in NetAccept that causes EBADF abort

[JakeChampion]

NetAcceptAction::cancel() closes the server socket before setting the cancelled flag
In the window between close and flag set, net_accept() can get EBADF from accept4(), see cancelled=false, and dispatch
EVENT_ERROR to handlers that don't expect it

now we check the atomic server pointer in all three accept paths before dispatching EVENT_ERROR

[Copilot]

🟡 Changes recommended

The listening-state check is not synchronized with cancellation, so the fatal accept-error race can still occur.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes a cancellation/accept race that can produce EBADF and fatal accept errors.

Changes:

• Adds atomic listening-server state tracking.
• Applies checks across all three accept error paths.
• Updates cancellation and socket ownership handling.

File summaries

File	Reviewed changes
src/iocore/net/UnixNetAccept.cc	Adds listening-state guards; cancellation remains unsynchronized with error dispatch, leaving the critical race unresolved.
src/iocore/net/P_NetAccept.h	Adds atomic server-state tracking and listening checks.

Review details

Suppressed comments (2)

src/iocore/net/UnixNetAccept.cc:390

• The is_listening() load is outside the mutex acquired on the next line. Cancellation can clear _server, close the socket, set cancelled, and release that mutex after this load but before this thread acquires it, so this branch can still deliver EVENT_ERROR to a cancelled continuation. Acquire the mutex before checking is_listening() (or recheck after acquiring it) so the check and callback are serialized with cancel().

        if (action_->is_listening()) {

src/iocore/net/UnixNetAccept.cc:583

• This path has no action/continuation mutex around the check or callback. is_listening() is only an atomic pointer load, so it can return true, then cancel() can exchange and close the server and set cancelled before the next line executes; EVENT_ERROR is still delivered after cancellation. The guard and callback need to be serialized with cancellation (or use a cancellation state that reserves callback delivery).

      if (action_->is_listening()) {

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[JosiahWI]

This fixes the issue described. I agree with @JakeChampion we should track each bug as its own patch.

I looked through the code to understand how the fix addresses the EBADF race specifically: the issue is that an EBADF from accept4 can result from side effects of an action cancellation that do not happen-before it. Therefore, a cancellation check in the code after the accept4 can observe no cancellation, misidentify the cause of the EBADF, and take the wrong logic path. The fix relies on the atomic server pointer to test for cancellation, insuring synchronization between cancellation and its test.

Briefly, the two remaining bugs I see are:

• stop_accept cancels the action without taking a mutex. Due to this fix, there is no data race concern, but this is a race condition because the cancellation can occur between the accept4 and the cancellation test, again violating assumptions in the logic.

• Some of the cancellation checks fixed in this PR happen directly before taking the action lock and immediately invoking its handler. This is also a race condition: the action can be cancelled after the check but before the callback. The code should be very close temporally, so the race condition should be very rare, but it would invoke a handler on a cancelled action, which is likely to cause a hard-to-diagnose crash.