Skip to content

Wake the killed thread, not another waiter, on a monitor wait - #9847

Open
sampokuokkanen wants to merge 1 commit into
jruby:masterfrom
sampokuokkanen:kill-wakes-right-waiter
Open

sampokuokkanen wants to merge 1 commit into
jruby:masterfrom
sampokuokkanen:kill-wakes-right-waiter

Conversation

@sampokuokkanen

Copy link
Copy Markdown
Contributor

waitForObject waits with Object.wait, and a kill or raise woke it with notify, which wakes one arbitrary waiter on that monitor. When two threads waited on the same object, the other one could wake instead, see nothing pending, and wait again, leaving the killed thread blocked. concurrent-ruby waits this way, which made its CyclicBarrier spec time out joining killed threads now and then.

notify() could wake some other waiter instead of the killed thread. notifyAll() wakes them all; the rest just recheck and go back to waiting.

Should fix the occasional CI error Concurrent::CyclicBarrier#number_waiting with waiting threads should be equal to the waiting threads count like here: https://github.com/jruby/jruby/actions/runs/39355604931/job/118409545094?pr=9845.

Related to ruby-concurrency/concurrent-ruby#862.

waitForObject waits with Object.wait, and a kill or raise woke it with
notify, which wakes one arbitrary waiter on that monitor. When two
threads waited on the same object, the other one could wake instead,
see nothing pending, and wait again, leaving the killed thread blocked.
concurrent-ruby waits this way, which made its CyclicBarrier spec time
out joining killed threads now and then.

Use notifyAll. Monitor waiters have to recheck their condition anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant