Skip to content

Commit 683d540

Browse files
ibraheemdevtaiki-e
authored andcommitted
Remove implicit clear in ReadyToRunQueue::drop (#2493)
1 parent f1c3678 commit 683d540

File tree

2 files changed

+4
-32
lines changed

2 files changed

+4
-32
lines changed

futures-util/src/stream/futures_unordered/mod.rs

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ impl<Fut> FuturesUnordered<Fut> {
558558
pub fn clear(&mut self) {
559559
self.clear_head_all();
560560

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

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

576576
impl<Fut> Drop for FuturesUnordered<Fut> {
577577
fn drop(&mut self) {
578-
// When a `FuturesUnordered` is dropped we want to drop all futures
579-
// associated with it. At the same time though there may be tons of
580-
// wakers flying around which contain `Task<Fut>` references
581-
// inside them. We'll let those naturally get deallocated.
582578
self.clear_head_all();
583-
584-
// Note that at this point we could still have a bunch of tasks in the
585-
// ready to run queue. None of those tasks, however, have futures
586-
// associated with them so they're safe to destroy on any thread. At
587-
// this point the `FuturesUnordered` struct, the owner of the one strong
588-
// reference to the ready to run queue will drop the strong reference.
589-
// At that point whichever thread releases the strong refcount last (be
590-
// it this thread or some other thread as part of an `upgrade`) will
591-
// clear out the ready to run queue and free all remaining tasks.
592-
//
593-
// While that freeing operation isn't guaranteed to happen here, it's
594-
// guaranteed to happen "promptly" as no more "blocking work" will
595-
// happen while there's a strong refcount held.
579+
// SAFETY: we just cleared all the tasks and we have &mut self
580+
unsafe { self.ready_to_run_queue.clear() };
596581
}
597582
}
598583

futures-util/src/stream/futures_unordered/ready_to_run_queue.rs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ impl<Fut> ReadyToRunQueue<Fut> {
9494
//
9595
// # Safety
9696
//
97-
// - All tasks **must** have had their futures dropped already (by FuturesUnordered::clear)
97+
// - All tasks **must** have had their futures dropped already (by FuturesUnordered::clear_head_all)
9898
// - The caller **must** guarantee unique access to `self`
9999
pub(crate) unsafe fn clear(&self) {
100100
loop {
@@ -107,16 +107,3 @@ impl<Fut> ReadyToRunQueue<Fut> {
107107
}
108108
}
109109
}
110-
111-
impl<Fut> Drop for ReadyToRunQueue<Fut> {
112-
fn drop(&mut self) {
113-
// Once we're in the destructor for `Inner<Fut>` we need to clear out
114-
// the ready to run queue of tasks if there's anything left in there.
115-
116-
// All tasks have had their futures dropped already by the `FuturesUnordered`
117-
// destructor above, and we have &mut self, so this is safe.
118-
unsafe {
119-
self.clear();
120-
}
121-
}
122-
}

0 commit comments

Comments
 (0)