utils/macos/epoll: Fix EV_EOF mapping#567
Merged
slp merged 1 commit intocontainers:mainfrom Mar 12, 2026
Merged
Conversation
Collaborator
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue in the macOS epoll shim where EV_EOF was not being mapped correctly to READ_HANG_UP and HANG_UP events. The change now properly uses the event filter type (EVFILT_READ or EVFILT_WRITE) to determine the appropriate hangup type. The associated debug logging and documentation have also been updated to reflect this change. My review includes a suggestion to refactor the new logic to improve its readability and reduce duplication.
nirs
requested review from
MatiasVara,
dorindabassey,
jakecorrenti,
mtjhrc,
slp and
tylerfanelli
as code owners
The code used EV_CLEAR (edge-trigger flag) as a proxy for the event direction when mapping EV_EOF to a hangup type. This produced the wrong type: backend WRITE+EOF got READ_HANG_UP, and eventfd READ+EOF got HANG_UP. No impact in practice since the worker treats both HANG_UP and READ_HANG_UP the same way (fatal error, disable networking). Use the filter type instead: - EVFILT_READ + EV_EOF → READ_HANG_UP (half-close) - EVFILT_WRITE + EV_EOF → HANG_UP (connection broken) Unlike epoll where EPOLLRDHUP must be explicitly registered, kqueue always reports EV_EOF. The shim does not track registration, so READ_HANG_UP is always reported on read EOF. Document this as a known difference from epoll in ctl(). Signed-off-by: Nir Soffer <nirsof@gmail.com>
nirs
force-pushed
the
macos-epoll-hangup
branch
from
740c9bd to
d67cf55
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The code used EV_CLEAR (edge-trigger flag) as a proxy for the event
direction when mapping EV_EOF to a hangup type. This produced the
wrong type: backend WRITE+EOF got READ_HANG_UP, and eventfd READ+EOF
got HANG_UP.
No impact in practice since the worker treats both HANG_UP and
READ_HANG_UP the same way (fatal error, disable networking).
Use the filter type instead:
Unlike epoll where EPOLLRDHUP must be explicitly registered, kqueue
always reports EV_EOF. The shim does not track registration, so
READ_HANG_UP is always reported on read EOF. Document this as a
known difference from epoll in ctl().
Based on #566