Ignore stale kqueue events for an already-deregistered key - #3502
Open
afonsojanu wants to merge 1 commit into
Open
Ignore stale kqueue events for an already-deregistered key#3502afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
In guest mode, get_events() can grab a batch of ready kqueue events ahead of when they actually reach process_events() on a later tick. notify_closing() can run in that window and remove the same key from _registered, since it's a plain synchronous call rather than something gated behind a checkpoint. When the stale event then shows up, process_events() looked it up with a bare dict subscript and blew up with a KeyError, which guest mode wraps into a fatal TrioInternalError. This matches issue python-trio#3500, where a PySide6 app running trio in guest mode occasionally crashed with KeyError: (46, -2) from exactly this path. Treat a missing key the same way the epoll backend already does: there's nothing left to wake up, so just move on to the next event.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3502 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 128 128
Lines 19452 19467 +15
Branches 1321 1322 +1
===============================================
+ Hits 19452 19467 +15
🚀 New features to boost your workflow:
|
Contributor
|
Please first write a reproducer that doesn't look into the attributes for the kqueue manager. You claim |
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
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.
Closes #3500.
With a PySide6 app running trio in guest mode,
process_events()in the kqueue backend can occasionally blow up withKeyError: (46, -2), which guest mode then wraps into a fatalTrioInternalError.The root cause is a timing gap between
get_events()andprocess_events(). In guest mode,GuestState.guest_tickfetches a batch of ready kqueue events with a non-blockingget_events(0)right after resuming the scheduler for the current tick, then hands that batch off to be delivered on the next tick, once the host loop gets around to scheduling it.notify_closing()is a plain synchronous function, not tied to a checkpoint, so it can run in that gap and remove the fd/filter key from_registeredbefore the already-fetched event for that same key gets delivered.process_events()looked the key up with a bareself._registered[key], so a stale event for a since-deregistered key crashed the whole run.The epoll backend doesn't have this problem because its
_registeredis adefaultdict, so a lookup for a missing key just returns an empty waiter set instead of raising. This PR gives kqueue the same tolerance: a missing key means there's nothing left to wake up, so the event is skipped.Added a regression test that constructs a
KqueueIOManagerdirectly, registers then deregisters a key (mimicking whatnotify_closingdoes), and feeds a matching event straight intoprocess_events(). It reproduces the exactKeyErroron the old code and passes with the fix. Ran the full_coretest suite locally on macOS (arm64), 277 passed.