Skip to content

Commit

Permalink
Remove implicit clear in ReadyToRunQueue::drop (#2493)
Browse files Browse the repository at this point in the history
  • Loading branch information
ibraheemdev committed Sep 2, 2021
1 parent f3dd0b9 commit 37dfb05
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 32 deletions.
21 changes: 3 additions & 18 deletions futures-util/src/stream/futures_unordered/mod.rs
Expand Up @@ -558,7 +558,7 @@ impl<Fut> FuturesUnordered<Fut> {
pub fn clear(&mut self) {
self.clear_head_all();

// we just cleared all the tasks, and we have &mut self, so this is safe.
// SAFETY: we just cleared all the tasks and we have &mut self
unsafe { self.ready_to_run_queue.clear() };

self.is_terminated.store(false, Relaxed);
Expand All @@ -575,24 +575,9 @@ impl<Fut> FuturesUnordered<Fut> {

impl<Fut> Drop for FuturesUnordered<Fut> {
fn drop(&mut self) {
// When a `FuturesUnordered` is dropped we want to drop all futures
// associated with it. At the same time though there may be tons of
// wakers flying around which contain `Task<Fut>` references
// inside them. We'll let those naturally get deallocated.
self.clear_head_all();

// Note that at this point we could still have a bunch of tasks in the
// ready to run queue. None of those tasks, however, have futures
// associated with them so they're safe to destroy on any thread. At
// this point the `FuturesUnordered` struct, the owner of the one strong
// reference to the ready to run queue will drop the strong reference.
// At that point whichever thread releases the strong refcount last (be
// it this thread or some other thread as part of an `upgrade`) will
// clear out the ready to run queue and free all remaining tasks.
//
// While that freeing operation isn't guaranteed to happen here, it's
// guaranteed to happen "promptly" as no more "blocking work" will
// happen while there's a strong refcount held.
// SAFETY: we just cleared all the tasks and we have &mut self
unsafe { self.ready_to_run_queue.clear() };
}
}

Expand Down
15 changes: 1 addition & 14 deletions futures-util/src/stream/futures_unordered/ready_to_run_queue.rs
Expand Up @@ -94,7 +94,7 @@ impl<Fut> ReadyToRunQueue<Fut> {
//
// # Safety
//
// - All tasks **must** have had their futures dropped already (by FuturesUnordered::clear)
// - All tasks **must** have had their futures dropped already (by FuturesUnordered::clear_head_all)
// - The caller **must** guarantee unique access to `self`
pub(crate) unsafe fn clear(&self) {
loop {
Expand All @@ -107,16 +107,3 @@ impl<Fut> ReadyToRunQueue<Fut> {
}
}
}

impl<Fut> Drop for ReadyToRunQueue<Fut> {
fn drop(&mut self) {
// Once we're in the destructor for `Inner<Fut>` we need to clear out
// the ready to run queue of tasks if there's anything left in there.

// All tasks have had their futures dropped already by the `FuturesUnordered`
// destructor above, and we have &mut self, so this is safe.
unsafe {
self.clear();
}
}
}

0 comments on commit 37dfb05

Please sign in to comment.