time: Fix race condition in timer drop #3229
Conversation
Dropping a timer on the millisecond that it was scheduled for, when it was on the pending list, could result in a panic previously, as we did not record the pending-list state in cached_when. Hopefully fixes: ZcashFoundation/zebra#1452
tokio/tests/time_sleep.rs
Outdated
| use std::sync::{Arc, Mutex}; | ||
| use std::task::Context; | ||
|
|
||
| let paniced = Arc::new(AtomicBool::new(false)); |
tokio/src/time/driver/wheel/mod.rs
Outdated
| unsafe { | ||
| item.set_cached_when(u64::max_value()); | ||
| } |
There was a problem hiding this comment.
There's a comment a few lines above (on the match) that seems to imply that mark_pending updates cached_when; this isn't actually the case. Maybe we should update that comment to indicate that this is where cached_when is updated?
There was a problem hiding this comment.
On second thought, mark_pending is probably the right place to do this update (it does this update when a timer is found to have been rescheduled)
There was a problem hiding this comment.
I was wondering about that too...this is the only place we call mark_pending, so it makes sense. Then, we can remove the set_cached_when methods.
Darksonn
added
A-tokio
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
|
@carllerche did you want to take a look at this before it merges? it looks good to me but i thought you might want to sign off... |
|
@hawkw I trust you 👍 |
Updates the tokio dependency to the commit that merged tokio-rs/tokio#3229, which should fix the time wheel panic in ZcashFoundation#1452.
Updates the tokio dependency to the commit that merged tokio-rs/tokio#3229, which should fix the time wheel panic in #1452.
|
I've been running |
|
@teor2345 great news. Thanks for checking 👍 |
Dropping a timer on the millisecond that it was scheduled for, when it was on
the pending list, could result in a panic previously, as we did not record the
pending-list state in cached_when.
Hopefully fixes: ZcashFoundation/zebra#1452
Motivation
Solution