-
Notifications
You must be signed in to change notification settings - Fork 332
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
Investigate "no callback found" #1763
Comments
The only idea that comes to my mind is that host's clock was shifted back just at the right moment… |
For the system clock that would actually be "legal". |
But yeah, I think the only way this can happen is if (Btw, do we compute the wait time on each iteration through the main interpreter loop? That seems like a lot, TBH.) |
I guess in that case instead of unwrapping we should go back to the scheduler without doing anything: let (thread, callback) = if let Some((thread, callback)) = this.machine.threads.get_ready_callback() {
(thread, callback)
} else {
// get_ready_callback can return None if the computer's clock was
// shifted after calling the scheduler and before the call
// to get_ready_callback. In this case, just do nothing, which
// effectively just returns to the scheduler.
return Ok(());
};
Yes. |
Yeah, that sounds reasonable.
Hm, now I wonder if it's a problem (in particular perf-wise) to keep hitting the clock API so rapidly. At least we only do it when there are timeouts waiting to happen, but then we do it once for each timeout. |
I guess, the scheduler, in general, could be called each 10th step or so. I remember that at some point there was an idea to change the scheduler to pick a different thread after some number of steps. However, this probably was not implemented, right? |
One naive approach would be to move the "can we continue running the current thread" check above the timeout -- but I think the comment there explains that we cannot do that, even though I do not fully understand it. |
I think we could move the continue the current thread check above the timeout check if we changed the implementation of conditional variables to immediately return |
Wouldn't that effect also be achieved by making |
I think that this should also work. |
#1761 managed to somehow trigger an ICE in Miri:
The panic ultimately triggered here:
miri/src/thread.rs
Line 714 in 7b2e325
I have not seen this before, so looks like it is unlikely but possible to happen. Would be good to at least come up with a plausible theory for a chain of events that lead here. Cc @vakaras
The text was updated successfully, but these errors were encountered: