Improve compatibility of handle_interrupt#6569
Merged
headius merged 15 commits intojruby:masterfrom Aug 2, 2021
Merged
Conversation
88c994e to
388fff1
Compare
When masked to avoid interrupts, any interrupt exceptions should be ignored unless there's an enqueued thread event like raise.
7844348 to
897579c
Compare
CRuby separates its thread events ("check ints") polling between
general polling and polling before a blocking call. This allows
only triggering certain events immediately before a blocking call
and not during regular code execution, such as to ensure that
only IO operations will trigger an interrupt. The change here
moves most pollThreadEvents calls to blockingThreadPoll, which
then checks for interrupts with blocking operations in mind. The
remaining calls to pollThreadEvents are reduced to non-blocking
polls, which will avoid them propagating interrupts intended only
for blocking operations.
Looping at this level provides no way to exit a task except through normal completion or an exceptional interrupt. In particular, this breaks lock-sleeping operations, since wakeup should resume execution but not in an exceptional way. This fixes a regression in MRI test_sync.rb's test_sync_lock_and_wakeup where it would run forever due to a "normal" wakeup being insufficient to exit this loop.
Misinterpreted logic from CRuby's vm_check_ints_blocking.
Because this utility function needs to poll for thread events before and after the task, we must also specify whether it should propagate "on blocking" events or not. This fixes the remaining Thread#handle_interrupt spec, which is untagged here.
A blocking pop cannot be interrupted by a normal wakeup, and when under an interrupt mask it cannot be interrupted by events that are masked. This change allows such blocking pops to retry when interrupted without a propagatable event, fixing the MRI test_thread.rb's test_handle_interrupt.
Kernel#p is supposed to be uninterruptible. This patch makes it so by adding a RubyThread utility function equivalent to CRuby's rb_uninterruptible. Also in this patch, Block gains three functional interfaces: Function and BiFunction from java.util.function and TriFunction now elevated to a public JRuby interface. This allows reusing the same handle_interrupt logic for Kernel#p. The FunctionOneOrTwoOrThree interface is provided to deal with incompatible return types in the default #andThen functions provided by Function, BiFunction, and TriFunction.
No interrupt set means the exception that should propagate immediately will propagate at the next poll, potentially far from the original raise.
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.
The behavior of
handle_interruptdiffers from CRuby in enough ways to cause some spec failures. None of these behavioral differences appear to affect the criticality ofhandle_interruptblocks but they should be fixed to get specs green and behavior consistent with CRuby.See #6544.