-
Notifications
You must be signed in to change notification settings - Fork 440
call unblock callbacks only when the thread is scheduled again #4828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
| // `pthread_cond_timedwait`, "an error is returned if [...] the absolute time specified by | ||
| // abstime has already been passed at the time of the call". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That has nothing to do with the timing of the callback – pthread_cond_timedwait is allowed to yield even if the time has passed already. The important thing is that the timeout checks happen before selecting the next thread, otherwise you could get a deadlock.
|
The logic for concurrency primitives such as mutexes relies on having the callback invoked immediately (for instance, in this assertion). So delaying this until the next time the thread is scheduled is not an option I think. I very deliberately chose the current design when I last refactored this code. |
Those sound similar to Linux's timerfd? What I kind of expected would happen there is that when a thread blocks on the FD, we compute when it has to wake up again and block the thread accordingly. That would avoid having the scheduler worry about any non-thread objects. Does that not work? |
They are quite similar. The problem is that another thread could change the timer time while threads are blocked, which would invalidate their timeout time – they'd need to recompute it somehow. Also, the current approach necessitates going over all threads twice before executing the timeout. For |
Ah, so the docs say that if the timer gets updated when threads are already sleeping, those threads might get woken up earlier or later or even immediately? I was trying to find that out but it wasn't clear to me. Is it the same for timerfd? Yeah that is more challenging to implement. I'll have to think about this. |
|
How did you plan to deal with this timeout-changed-when-thread-is-already-blocked situation? I don't see how "delay callback until thread gets scheduled again" makes that part easier.
It's not supposed to run any code (as in, MIR), but these unblock callbacks aren't code, they are just interpreter internals. The scheduler already runs unblock callbacks when timeouts occur and that's fine.
I don't know what you are referring to here. |
I'm trying to shim waitable timer objects since I want to make use of them in
std(see rust-lang/rust#151553). My idea is to modify the centralschedulefunction to not only look for timed out blocking operations but also go through all active timers and look for ones whose deadline has passed. This however necessitates waking up multiple threads fromschedule– which currently would run their wakeup callbacks, violating the principle thatscheduleis not supposed to run things. I could try to work my way around this, but I think a more principled solution is the following:This PR changes the scheduling of callback functions: Instead of being immediately invoked when an operation unblocks a thread, the thread is instead put into the newly created
ThreadState::Unblocked. Threads withThreadState::Unblockedare eligible for scheduling, but will, upon being selected, execute a callback instead of executing the nextstep.This has the nice side-effect of exploring more possible executions of the macOS futex functions – these retrieve the current futex waiter count in their wakeup callback, so by making this callback depend on scheduling, there are more possible values that can be returned. If I read the XNU sources correctly, that also matches the system behaviour (the waiter count is retrieved in
ulock_wait_cleanupwhich is called from theulock_wait_continuecontinuation).I have no clue about how this interacts with GenMC, that could potentially cause issues with the
BlockReason::GenMChack.