[resetius]

Problem

The poller could resume a coroutine handle that was stale: its frame already destroyed, or the coroutine already resumed for the same registration. This matches a SIGSEGV seen in an HTTP server on TLoop<TEPoll> (a jump through a freed coroutine frame into glibc's main_arena).

• Socket awaitables (ReadSome/WriteSome/Monitor, Accept, Connect) left their handle in Changes_, ReadyEvents_ and the backend tables when the coroutine was destroyed while suspended: RemoveEvent(THandle) was a no-op.
• epoll: closing a descriptor whose open file is still referenced elsewhere (a forked child, a dup) does not remove its epoll registration, and the deferred EPOLL_CTL_DEL fails with EBADF. Its events keep arriving with the old data.fd, which may already belong to a new socket: the new waiter is resumed spuriously, or twice in one iteration when both files are ready, and the second resume hits a freed frame.
• kqueue: on EOF each filter's event also woke the other direction's waiter, so with a reader and a writer on one descriptor both were resumed twice.
• poll: removing two slots in one iteration could leave an index in InEvents_ pointing past Fds_.size(); a later registration of that descriptor was written out of bounds and never polled (lost wakeup).

Fix

• New TPollerBase::RemoveEvent(int fd, THandle h). It drops h from ready events and queues a cancellation (TEvent::CANCEL) that each backend turns (ResolveCancel) into a removal of only the directions still registered to h, so a newer waiter on a reused descriptor number keeps its registration. The old no-op RemoveEvent(THandle) stays as a deprecated overload.
• The awaitables remember their handle while suspended and call it from their destructors. So the poller must now outlive every coroutine suspended on a socket awaitable, not only the open sockets; this is documented on TLoop and RemoveEvent.
• epoll stores a per-descriptor generation next to the fd in epoll_data and ignores events of older registrations.
• kqueue wakes only the filter's own waiter on EOF.
• poll pops a removed slot immediately.

The hot path gains one branch per change and one compare per epoll event.

Tests

New tests in tests/tests.cpp for select, poll, epoll and kqueue:

• test_destroyed_waiter_not_resumed: read and write waiters destroyed before the first poll, after registration, after being collected as ready, from a timer in the same iteration, and replaced by a new waiter.
• test_destroyed_accept_and_connect_not_resumed: the same for Accept and Connect, plus a Connect timeout in the iteration where the socket becomes writable (covered by the RemoveEvent(fd) from Report connect errors from Connect #141).
• test_reused_descriptor_wakes_new_waiter: the descriptor number is reused before the next poll, with and without the old file kept open elsewhere.
• test_reuse_after_closing_several: two registered descriptors closed in one iteration, then reused.
• test_hangup_wakes_reader_and_writer_once: a reader and a writer on one socket when the peer closes.
• test_short_connections: short and aborted connections, with accepted sockets kept open for a while as a forked child would; every complete request must get its response.

On master with ASan:

test	select	poll	epoll	kqueue
test_destroyed_waiter_not_resumed	use-after-free	use-after-free	use-after-free	use-after-free
test_destroyed_accept_and_connect_not_resumed	use-after-free	use-after-free	use-after-free	use-after-free
test_reused_descriptor_wakes_new_waiter	pass	pass	use-after-free	pass
test_reuse_after_closing_several	pass	hangs (lost wakeup)	pass	pass
test_hangup_wakes_reader_and_writer_once	pass	pass	pass	use-after-free
test_short_connections	pass	out of bounds write in Fds_	use-after-free	pass

(epoll from Linux CI on the tests-only revision, kqueue on macOS, select and poll on both.) All pass with the fix.

[github-actions]

Unit Test Results (Linux)

197 tests  +18   197 ✅ +18   15s ⏱️ ±0s
  4 suites ± 0     0 💤 ± 0 
  4 files   ± 0     0 ❌ ± 0 

Results for commit 0b81655. ± Comparison against base commit cae2aef.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Results (MacOS)

164 tests  +18   164 ✅ +18   26s ⏱️ +5s
  4 suites ± 0     0 💤 ± 0 
  4 files   ± 0     0 ❌ ± 0 

Results for commit 0b81655. ± Comparison against base commit cae2aef.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Results (Windows)

135 tests  ±0   135 ✅ ±0   39s ⏱️ ±0s
  4 suites ±0     0 💤 ±0 
  4 files   ±0     0 ❌ ±0 

Results for commit 0b81655. ± Comparison against base commit cae2aef.

♻️ This comment has been updated with latest results.

[resetius]

Independent review of head d5ad9b9f09f215ce163ab02cfc9a3ac67323b200. Verdict: approve. No P1 or P2 findings. Five P3 notes below, one of them (epoll) needs an owner decision.

What was checked

1. Lifecycle of a waiter (select, poll, kqueue run locally with ASan; epoll from CI). I traced these paths: destroyed before the first Poll, after registration, after its event was collected, by a timer in the same iteration, from inside another waiter's resume (the Any() pattern), replaced by a new waiter on the same direction, its socket closed and then destroyed, and its socket closed, the fd reused by a new waiter, and then destroyed. I also traced Connect with timer and event in the same iteration, and the interaction with RemoveEvent(int fd) from #141.

• Cancellations are resolved in Changes_ order. A newer waiter on the same fd number keeps its registration. So does a new frame allocated at the same address, because it registers after the destruction.
• TEvent::Match never matches Type 0, so the re-registration check in Wakeup is not affected.
• RemoveEvent(fd) turns a queued cancellation into {fd, 0, {}}, which every backend skips.

To test this I wrote a randomized model checker. It uses 6 socketpairs and random reader and writer waiters, with the frame registered in the poller freed on completion, so a second resume is a use-after-free. Random actions: destroy while suspended, replace, close with or without destroying the waiters, reopen (fd reuse), timer-driven and in-resume actions, and Poll() / action / WakeupReadyHandles() splits. A periodic liveness check requires that every live waiter whose fd is made ready completes within 20 steps, and that waiters left on closed fds are never resumed.

• Head: 600 seeds x 20000 iterations x {select, poll, kqueue}, 0 failures. Totals: 3.43M resumes, 4.15M waiters destroyed while suspended, 1.51M waiters left on closed fds, 2.75M fd reuses, 8.3M timer-driven actions, 680k actions from inside a resume.
• Master 45c931a: heap-use-after-free in TPollerBase::Wakeup on seed 1 for all three backends.

2. epoll generation.

• Layout: the fd is in the low 32 bits of data.u64 and the generation in the high 32 bits. The fd is non-negative, so the uint32 round trip is exact. The generation wraps only after 2^32 registrations of one fd while a stale registration from exactly that generation is still alive.
• ADD bumps the generation. MOD keeps it. MOD that fails with ENOENT and falls back to ADD bumps it. DEL leaves it unchanged: stale events that still carry the current generation find InEvents_[fd] empty and do nothing, and the next ADD bumps it.
• Generations_ is resized together with InEvents_, so indexing by the decoded fd stays in bounds.
• EEXIST on ADD is possible only if the same open file comes back at the same number (dup2, SCM_RIGHTS). That is pre-existing.
• wepoll: epoll_data_t has u64 and wepoll returns it unchanged. wepoll also drops closed sockets by itself (AFD_POLL_LOCAL_CLOSE). Windows CI builds and passes, and the existing my_unit_poller tests run TEPoll on Windows. The new tests are under #ifndef _WIN32, which is why the Windows count is 129 +-0.

3. uring and iocp.

• A Type-0 change comes only from the destructor of a TSocketBase / TSocket awaitable that actually suspended.
• TUring::TSocket and TUring::TFileHandle are the poller-driven classes. They hide ReadSome, WriteSome, the *Yield variants, Accept and Connect with awaitables that have no destructor. The only inherited awaitable is Monitor, which never suspends (Monitor() never suspends: the awaitable completes immediately instead of waiting for the remote hang-up #144). Using NNet::TSocket directly with TUring already trips the same assert in TUring::Wait via AddRead.
• TIOCp::Poll drops Changes_ in Reset() before waiting.
• Conclusion: neither backend can see Type 0. Linux CI found liburing and ran the my_unit_poller tests with TUring under ASan; they pass.

4. kqueue and poll.

• kqueue: each filter reports EV_EOF itself. I checked this with TCP, not only AF_UNIX: a reader and a writer on one TCP socket with a full send buffer, and the peer goes away in three ways (close with unread data, SO_LINGER 0 close, shutdown(SHUT_WR) followed by close 200 ms later).
  • Head: reader and writer each resumed exactly once on select, poll and kqueue. Dropping the cross-wake loses no writer wakeup.
  • Master: heap-use-after-free in Wakeup on kqueue.
  • A half-close no longer wakes the writer spuriously.
• poll: popping the slot immediately keeps the InEvents_ indexes consistent, including the case where the slot swaps with itself.
• The known RHUP issues (TPoll on Linux stores the RHUP handle in ev.Write instead of ev.RHup #143, Monitor() never suspends: the awaitable completes immediately instead of waiting for the remote hang-up #144, kqueue (and select, non-Linux poll) silently ignore AddRemoteHup registrations #146) are untouched and not made worse.

5. Hot path. No map or unordered_map, and no per-event or per-change allocation.

• Per change: Resolve() costs two compares and a 16-byte TEvent copy; a cancellation adds three handle compares. epoll adds if (!ch.Type), plus a shift/or per ADD or MOD.

• Per epoll event: one 4-byte load from Generations_ (a separate vector, so possibly one more cache line) and a compare.

• Memory: 8 bytes per socket awaitable, 4 bytes per fd for epoll.

• Per cancellation: an O(|ReadyEvents_|) scan, one push into Changes_ (capacity is reused), and up to two extra epoll_ctl calls if the waiter is cancelled before its registration is applied.

• Measured with examples/bench.cpp at -O2: 1000 socketpairs, 100 active, 100k writes, p50 of 25 runs, 3 alternating rounds per backend.

  backend	master p50 (ms)	head p50 (ms)	change
  kqueue	45.5-45.8	45.7-45.9	+0.3%
  poll	143.7-147.9	144.4-147.9	within noise
  select (400 pairs)	50.7-51.0	51.1-51.2	+0.5%

  epoll was not measured.

6. Tests.

• New tests against master 45c931a library code, macOS with ASan: the PR's table is reproduced exactly for select, poll and kqueue. For poll, test_reuse_after_closing_several fails at the 5 s Complete deadline; the table says "hangs".
• epoll on master: the PR's tests-only CI run 41748455206 matches the table.
• Head: all 18 new cases pass locally. CI is green: Linux 191 (+18), macOS 158 (+18), Windows 129 (+-0).
• Determinism:
  • 12 sequential ctest runs: all pass.
  • 12 full ut_tests runs under 24 CPU-burning processes: all pass, 3.8-4.6 s each.
• Runtime is bounded: 5 s deadlines in Complete, the hangup test and test_short_connections, plus a 5 s drain. One robustness issue is listed below.

P3

1. epoll: a stale kernel registration is filtered but not removed (owner decision; not verified locally, the epoll path needs Linux). coroio/socket.hpp:259-266 closes the fd first and only then queues RemoveEvent(fd). The deferred EPOLL_CTL_DEL at coroio/backends/epoll.cpp:116-122 then fails with EBADF or ENOENT, which is ignored. If the open file is still referenced elsewhere (a forked child before exec, a dup), the old registration stays in the epoll set and stays level-triggered. The generation check at epoll.cpp:150-156 drops its events, but epoll_wait keeps returning them immediately: EPOLLHUP and EPOLLERR are always in the mask, and EPOLLOUT is too if a writer was registered. So the loop spins at full CPU for as long as the other holder keeps the file open and ready (epoll(7), Q6). This is not a regression: master spins as well, and also misdelivers those events. With CLOEXEC (Set close on exec for sockets #137) the window is fork-to-exec. Issuing EPOLL_CTL_DEL before close() would remove the cause, and the generation would then remain as a defence.

2. New lifetime requirement. The destructors added at coroio/socket.hpp:271-276, 427-434 and 504-508 call into the poller. The poller must now outlive every coroutine suspended on a socket awaitable; before, only open sockets needed that. Repro:

   std::optional<TFuture<void>> wait;
   int fds[2]; socketpair(AF_UNIX, SOCK_STREAM, 0, fds);
   {
       TLoop<TPoll> loop;
       TFileHandle handle(fds[0], loop.Poller());
       wait.emplace(ReadByte(handle));   // co_await handle.ReadSome(&b, 1)
       loop.Step();
   }
   wait.reset();

   • Head: AddressSanitizer: stack-use-after-scope ... in NNet::TPollerBase::RemoveEvent(int, std::coroutine_handle<void>) poller.hpp:150.
   • Master: no error.

   This is probably acceptable. One sentence in the RemoveEvent / TLoop docs would cover it.

3. test_short_connections exhausts ephemeral ports when looped. The test makes 900 loopback connections per run (300 per poller). The dup'd "inherited" descriptors (tests/tests.cpp:2051) delay the server's FIN, so the client closes first and its ephemeral port goes into TIME_WAIT. Running ut_tests --filter 'test_short_connections*' back to back on macOS failed at run 19 with 16362 sockets in TIME_WAIT:

   libc++abi: terminating due to uncaught exception of type std::__1::system_error: connect: Can't assign requested address
   

   The exception escapes reap() (tests/tests.cpp:2088-2091) and aborts the whole binary instead of failing one test. Single runs are fine, but ctest --repeat or parallel jobs on one host will trip it. Options: SO_LINGER {1, 0} on the client socket, fewer requests, or catching the exception in reap.

4. Design nits.

   • Type 0 as the "cancel h" marker is implicit, and Wakeup relies on TEvent::Match never matching it (coroio/base.hpp:53-55). A named constant or a comment on TEvent would make that visible.
   • Resolve is a generic name for what it does; ResolveCancel or similar would say more.
   • Removing the public RemoveEvent(THandle) is a source break for any external caller, even though it was a no-op. Worth a line in the release notes.
   • Optional: keeping the generation inside the InEvents_ entry would save the second array access per epoll event.

5. Coverage gaps (not bugs). wepoll gets no new tests, because the new tests are non-Windows. Cancellation of ReadSomeYield / WriteSomeYield is not tested directly; they share the TAwaitable base. Monitor cancellation cannot be reached until Monitor() never suspends: the awaitable completes immediately instead of waiting for the remote hang-up #144 is fixed.

[resetius]

Independent review of head d82dd5253292e18366584e1cd9192026c5a5372a (delta from d5ad9b9). Verdict: approve, no P1/P2 findings. The earlier review of d5ad9b9 still applies.

The rebase onto cae2aef leaves the library part of this PR's diff unchanged. I compared the two heads with git range-diff 45c931a..d5ad9b9 cae2aef..d82dd52: every difference is in tests/tests.cpp.

• getport() was renamed to NNet::GetFreePort() in test_destroyed_accept_and_connect_not_resumed and in test_short_connections.
• SendRequest now splits the request at the header terminator: it sends ...HTTP/1.1\r\nHost: x\r\n, then \r\n. Before, the split was inside a header line (Ho + st: x\r\n\r\n). The server still waits for the second part, as before, and the test now also exercises the split-delimiter case fixed by Find ReadUntil delimiter split between reads #142.

Checked locally on d82dd52 (macOS, ASan): the build succeeds, ctest passes 4/4, and all 18 new cases (select, poll, kqueue) pass. CI on d82dd52 is green: Linux 197 (+18), macOS 164 (+18), Windows 135 (+-0).

P3 notes 1-5 from the previous review stand. P3 #3 (test_short_connections exhausts ephemeral ports when looped) is not changed by the rebase.

[resetius]

Independent review of head 0b81655275b59c27889e0a0e28e4ce678d01585c (delta from d82dd52). Verdict: approve. The delta adds no P1/P2 findings. It addresses P3 notes 2, 3 and 4 from my earlier review; P3 note 1 is now issue #150.

Checked with git diff d82dd52 0b81655:

• Behaviour is unchanged. TEvent::CANCEL = 0, so ResolveCancel's change.Type != TEvent::CANCEL is the old change.Type test, and TEvent{fd, TEvent::CANCEL, h} is the old {fd, 0, h}. The four backends only rename Resolve to ResolveCancel. Everything else in the library is documentation: the @note in loop.hpp and the extra sentence on RemoveEvent(int, THandle).
• No ambiguity from the deprecated RemoveEvent(THandle). I compiled a probe with clang and GCC 16 at -Wall -Wextra.
  • These resolve to RemoveEvent(int) with no warning: int, unsigned, a uintptr_t (SOCKET-like), short, char, 0 and {}.
  • These resolve to the deprecated no-op and warn: a THandle, a std::coroutine_handle<TPromise<void>> and nullptr.
  • RemoveEvent(5, h) still selects the cancellation.
  • Nothing in the library calls the deprecated overload, and the build has no warnings.
• The test loop is robust now. Clients close with SO_LINGER {1,0}. Locally on macOS with ASan:
  • 60 back-to-back runs of ut_tests --filter 'test_short_connections*': all pass, with 42 sockets in TIME_WAIT afterwards. Before the change, this loop failed on run 19.
  • 25 back-to-back runs of the full ut_tests: all pass, with 296 sockets in TIME_WAIT afterwards.
  • 3 parallel workers x 30 runs of the new tests: all pass.
  • reap() now reports an escaped exception through fail_msg, so only that test fails.
• Do the tests still catch the bugs on master? I ran the head's tests against master cae2aef library code. test_short_connections(TPoll) still fails with an ASan container-overflow in TPoll::Poll. The other new tests also fail on master exactly as listed in the PR table. For epoll I could not run anything (macOS only), and no tests-only CI run exists for the RST variant, so whether test_short_connections(TEPoll) still fails on master is unverified. With RST the server's kept files stay permanently EPOLLERR|EPOLLHUP-ready, so their stale registrations fire on every epoll_wait. Either way, the epoll stale-registration case stays pinned by test_reused_descriptor_wakes_new_waiter(TEPoll) (the shared variant), which this delta does not touch and which failed on master in CI run 41748455206.
• CI on 0b81655 is green: Linux 197 (+18), macOS 164 (+18), Windows 135 (+-0).