From 9080b83b40d55a4268f2ae44aaeed2adb9fdfa04 Mon Sep 17 00:00:00 2001
From: Ibraheem Ahmed <ibrah1440@gmail.com>
Date: Thu, 2 Sep 2021 13:59:20 -0400
Subject: [PATCH] Remove implicit clear in `ReadyToRunQueue::drop` (#2493)

---
 .../src/stream/futures_unordered/mod.rs       | 21 +++----------------
 .../futures_unordered/ready_to_run_queue.rs   | 15 +------------
 2 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/futures-util/src/stream/futures_unordered/mod.rs b/futures-util/src/stream/futures_unordered/mod.rs
index 4a05d8823e..6918a26b91 100644
--- a/futures-util/src/stream/futures_unordered/mod.rs
+++ b/futures-util/src/stream/futures_unordered/mod.rs
@@ -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);
@@ -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() };
     }
 }
 
diff --git a/futures-util/src/stream/futures_unordered/ready_to_run_queue.rs b/futures-util/src/stream/futures_unordered/ready_to_run_queue.rs
index 5ef6cde83d..2bc208682a 100644
--- a/futures-util/src/stream/futures_unordered/ready_to_run_queue.rs
+++ b/futures-util/src/stream/futures_unordered/ready_to_run_queue.rs
@@ -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 {
@@ -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();
-        }
-    }
-}