fix: Deadlocks in singlethreaded execution context #3860
Conversation
|
So, no need for user to make their own interface after all :) |
|
|
||
| import scala.scalanative.annotation.alwaysinline | ||
| import scala.concurrent.duration.FiniteDuration | ||
|
|
||
| object Proxy { |
There was a problem hiding this comment.
Shouldn't this also be private[scala.scalanative]?
At least based on the description from #3863, I thought each project that wanted to use this would have to define their own Proxy
There was a problem hiding this comment.
Right, this one should be fixed, but I'll do it in follow up PR. It's actually more dangerous since it's defined in javalib which does not emit Bytecode
| val deadline = now + timeout.toNanos | ||
| Proxy.stealWork(timeout) |
There was a problem hiding this comment.
This probably won't really matter (it's not like the toNanos conversion will take 1ms, and that doSleep is not exactly precise...), but just in case, it might be worth to run the stealWork ASAP.
| val deadline = now + timeout.toNanos | |
| Proxy.stealWork(timeout) | |
| Proxy.stealWork(timeout) | |
| val deadline = now + timeout.toNanos |
|
|
||
| @alwaysinline | ||
| final def parkNanos(nanos: Long): Unit = if (nanos > 0) { | ||
| park(nanos, isAbsolute = false) | ||
| if (isMultithreadingEnabled) park(nanos, isAbsolute = false) | ||
| else NativeExecutionContext.queueInternal.stealWork(nanos.nanos) |
There was a problem hiding this comment.
I'm not sure if this works as expected, as stealWork(FiniteDuration) rounds down to the millisecond, so in a lot of situations this won't yield.
I guess this matches what's said on the javadoc, but I also see that the WindowsThread uses millis.max(1).toUInt... I don't know, maybe this is OK. 🤷
There was a problem hiding this comment.
If we use JDK as reference implementation I’d like to point that small delay is a kind of edge case.
see: https://kirill.korins.ky/articles/sleeping-at-jvm-thread-for-a-very-short-period/ where I kept notes on that investigation
…make stealWork yield for deadline < 1ms
|
@WojciechMazur I have one bad news. This PR doesn't fix #3848 |
Fixes #3859
Based on work in #3853, kept as draft until deciding the new API for Native ExecutionContext
The only changes related to this fix are in the commit 516444b