Kernel#sleep swallows InterruptedException #4206
Comments
|
I think you're right. This also was reported in another bug...I'll try to find that. |
|
The InterruptedException is checked, so it still needs to be caught...but we'll just break out of the spin loop if it is raised. I have a fix coming...writing up a test now. |
|
I guess that'll work, instead of trying to rescue InterruptedException, I can just check the native thread for isInterrupted() right after the sleep exits if I need to do something specific in that case. |
|
Yeah sorry...we can't really allow the InterruptedException to get out, and there's no equivalent exception that MRI throws here for interrupted sleeps. The closest equivalent behavior in Ruby would be Thread#wakeup, which interrupts the sleep but does not raise an error. |
|
Right, that's exactly what I realized after I read that 👍 |
|
If the user needs to check isInterrupted() before returning, that should probably be done in a wrapper method of the same class for convenience (even if the wrapped method also needs to be public). Compare this method from Guava Testlib: https://github.com/google/guava/blob/794a10a4495fd7cd554eb0a9e480708d0277aced/guava/src/com/google/common/util/concurrent/Uninterruptibles.java#L382-L402 |
|
@Pr0methean Yeah I agree, it would be better to have an explicit uninterruptible sleep, but we're somewhat limited by what CRuby supports here. All the publicly callable sleep functions go through logic that checks for spurious wakeup and goes back to sleep. They do have C functions that will allow spurious wakeup, but as far as I can tell this is only used for utilitarian sleeps like in between polling a pid for completion. |
In order for users to be able to test if the thread's sleep loop was interrupted, we need to ensure the interrupt bit is set after an interrupted loop, even if the loop itself ignores the interrupt. See jruby#4206.
|
@Pr0methean I've pushed a patch in #5845 that re-interrupts the thread after the loop. I do have concerns though, since this sends the thread on its merry way with an interrupt bit just waiting to strike. Do you know if there's some written justification for this re-interrupting logic when doing a spin loop for uninterruptible sleep? |
|
So that the next interruptible operation will handle the interrupt. |
Currently, the Kernel#sleep code silently swallows any InterruptedException thrown by the native Java Thread.sleep(). Presumably, this was done in an attempt to prevent "spurious wakeups" from prematurely waking up sleeping threads. @headius has an old gist out there that was intended to remove this (https://gist.github.com/headius/5393700), but it simply gets rid of the entire spin lock and just calls the native sleep(). I think the spin lock is still necessary to account for spurious wakeups, but it shouldn't catch the InterruptedException, as those are only generated by Thread.interrupt(). According to the Java documentation, a spurious wakeup it truly spurious: it generates no exception or timeout, the sleeping thread simply stops sleeping.
I think a good solution would be to change the spin lock code in RubyKernel#sleep (currently starting at line 660) to this:
The original code wraps a try/catch around the sleep, which is what swallows the InterruptedException. Also, it breaks if sleep returns false... which seems to actually do the opposite of the intended behavior. RubyThread#sleep returns false if the sleep was bounded and did not sleep for the correct amount of time (i.e. spurious wakeup). Breaking from the while loop here when that happens would allow the spurious wakeup to terminate the sleep early. A spurious wakeup should not cause the sleep to terminate early, and an InterruptedException should bubble up, since those are quite intentional.
The text was updated successfully, but these errors were encountered: