-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix panics on updating DelayQueue entries #3270
Changes from all commits
5507728
7653585
8fac182
736de3c
09f8daf
053effe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,9 +116,17 @@ where | |
Ok(()) | ||
} | ||
|
||
/// Remove `item` from thee timing wheel. | ||
/// Remove `item` from the timing wheel. | ||
pub(crate) fn remove(&mut self, item: &T::Borrowed, store: &mut T::Store) { | ||
let when = T::when(item, store); | ||
|
||
assert!( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By the way, I added this assert because while the assert in I added the same condition as a |
||
self.elapsed <= when, | ||
"elapsed={}; when={}", | ||
self.elapsed, | ||
when | ||
); | ||
|
||
let level = self.level_for(when); | ||
|
||
self.levels[level].remove_entry(when, item, store); | ||
|
@@ -240,9 +248,11 @@ where | |
} | ||
|
||
fn level_for(elapsed: u64, when: u64) -> usize { | ||
let masked = elapsed ^ when; | ||
const SLOT_MASK: u64 = (1 << 6) - 1; | ||
|
||
assert!(masked != 0, "elapsed={}; when={}", elapsed, when); | ||
// Mask in the trailing bits ignored by the level calculation in order to cap | ||
// the possible leading zeros | ||
let masked = elapsed ^ when | SLOT_MASK; | ||
|
||
let leading_zeros = masked.leading_zeros() as usize; | ||
let significant = 63 - leading_zeros; | ||
|
@@ -255,7 +265,7 @@ mod test { | |
|
||
#[test] | ||
fn test_level_for() { | ||
for pos in 1..64 { | ||
for pos in 0..64 { | ||
assert_eq!( | ||
0, | ||
level_for(0, pos), | ||
|
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.
So, just to confirm, the reason this is correct is that the call to
insert_idx
will set it back totrue
if it is expired?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.
Yes,
remove_key
combined with these assignments should leave the slab entry in the same state as if it was a newly createdData
value ininsert_at
. Theninsert_idx
is responsible for placing the entry in the wheel or expired queue and setting the flag.