Mutex fixes for interruptibility#5942
Merged
headius merged 2 commits intojruby:masterfrom Oct 26, 2019
Merged
Conversation
The original code would immediately error out if a contended lock acquisition were interrupted before it could acquire the lock. This led to a behavioral difference from MRI, which does not raise errors. It is not clear whether that's because locks in MRI can never be truly contended (if the lock is not available the thread is put to sleep and woken by the lock owner) or because they explicitly try again to acquire the lock after waking. In this version of the code, we opt to continue trying to acquire the lock via a loop, unless a thread event like raise or kill tell us to divert from the normal path of execution. The only possible outcomes from interrupting a lock acquisition are to keep trying until we have the lock or error out. This patch opts for the former. Fixes jruby#5875.
Member
Author
|
Fix for #5875 is in. |
Previous logic used the same semaphore to sleep as any other sleep which interfered with code that expected the Mutex to be the lock used. The new logic uses the Mutex's JDK Lock, via a Condition, to do the sleeping. Because it is not possible for us to change the artificial thread status we maintain to "sleep" after the lock is released, this modified logic also introduces a new thread state that indicates that the native JDK thread state should be used. This gets closer to avoiding the racing status. It does not appear to eliminate the race altogether. Fixes jruby#5863.
1ed4081 to
f6a9b46
Compare
Member
Author
|
Fix for #5863 is in. The second fix introduces a new RubyThread status called "NATIVE", which indicates that the native JDK thread status should be translated into a Ruby thread status. This allows us to use the provided JDK Condition.await, which does the appropriate lock release followed by going into a wait state (via LockSupport.park). This in turn allows the specs to pass that depend on the lock being released by the time the thread status is "sleep". This fix is also a stepping stone toward eliminating the synthetic Ruby thread statuses. As we go forward we can modify more of these state changes to just defer to the NATIVE status, leaning on the JDK to handle state changes without racing. |
Member
Author
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.
This PR contains fixes for some interruptibility issues introduced in JRuby 9.2.8:
ConditionVariablewould be uninterruptible.WIP until I confirm it's ready.