Do not iterate elements in bounded queue/channel's destructor when ne…

…eds_drop returns false
crossbeam-rs · Dec 24, 2023 · cc65773 · cc65773
1 parent 60c3372
commit cc65773
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 56 deletions.
diff --git a/crossbeam-channel/src/flavors/array.rs b/crossbeam-channel/src/flavors/array.rs
@@ -9,7 +9,7 @@
 //!   - <https://docs.google.com/document/d/1yIAYmbvL3JxOKOjuCyon7JhW4cSv1wy5hC0ApeGMV9s/pub>
 
 use std::cell::UnsafeCell;
-use std::mem::MaybeUninit;
+use std::mem::{self, MaybeUninit};
 use std::ptr;
 use std::sync::atomic::{self, AtomicUsize, Ordering};
 use std::time::Instant;
@@ -520,36 +520,38 @@ impl<T> Channel<T> {
 
 impl<T> Drop for Channel<T> {
     fn drop(&mut self) {
-        // Get the index of the head.
-        let head = *self.head.get_mut();
-        let tail = *self.tail.get_mut();
-
-        let hix = head & (self.mark_bit - 1);
-        let tix = tail & (self.mark_bit - 1);
-
-        let len = if hix < tix {
-            tix - hix
-        } else if hix > tix {
-            self.cap - hix + tix
-        } else if (tail & !self.mark_bit) == head {
-            0
-        } else {
-            self.cap
-        };
-
-        // Loop over all slots that hold a message and drop them.
-        for i in 0..len {
-            // Compute the index of the next slot holding a message.
-            let index = if hix + i < self.cap {
-                hix + i
+        if mem::needs_drop::<T>() {
+            // Get the index of the head.
+            let head = *self.head.get_mut();
+            let tail = *self.tail.get_mut();
+
+            let hix = head & (self.mark_bit - 1);
+            let tix = tail & (self.mark_bit - 1);
+
+            let len = if hix < tix {
+                tix - hix
+            } else if hix > tix {
+                self.cap - hix + tix
+            } else if (tail & !self.mark_bit) == head {
+                0
             } else {
-                hix + i - self.cap
+                self.cap
             };
 
-            unsafe {
-                debug_assert!(index < self.buffer.len());
-                let slot = self.buffer.get_unchecked_mut(index);
-                (*slot.msg.get()).assume_init_drop();
+            // Loop over all slots that hold a message and drop them.
+            for i in 0..len {
+                // Compute the index of the next slot holding a message.
+                let index = if hix + i < self.cap {
+                    hix + i
+                } else {
+                    hix + i - self.cap
+                };
+
+                unsafe {
+                    debug_assert!(index < self.buffer.len());
+                    let slot = self.buffer.get_unchecked_mut(index);
+                    (*slot.msg.get()).assume_init_drop();
+                }
             }
         }
     }

diff --git a/crossbeam-queue/src/array_queue.rs b/crossbeam-queue/src/array_queue.rs
@@ -6,7 +6,7 @@
 use alloc::boxed::Box;
 use core::cell::UnsafeCell;
 use core::fmt;
-use core::mem::MaybeUninit;
+use core::mem::{self, MaybeUninit};
 use core::panic::{RefUnwindSafe, UnwindSafe};
 use core::sync::atomic::{self, AtomicUsize, Ordering};
 
@@ -447,36 +447,38 @@ impl<T> ArrayQueue<T> {
 
 impl<T> Drop for ArrayQueue<T> {
     fn drop(&mut self) {
-        // Get the index of the head.
-        let head = *self.head.get_mut();
-        let tail = *self.tail.get_mut();
-
-        let hix = head & (self.one_lap - 1);
-        let tix = tail & (self.one_lap - 1);
-
-        let len = if hix < tix {
-            tix - hix
-        } else if hix > tix {
-            self.cap - hix + tix
-        } else if tail == head {
-            0
-        } else {
-            self.cap
-        };
-
-        // Loop over all slots that hold a message and drop them.
-        for i in 0..len {
-            // Compute the index of the next slot holding a message.
-            let index = if hix + i < self.cap {
-                hix + i
+        if mem::needs_drop::<T>() {
+            // Get the index of the head.
+            let head = *self.head.get_mut();
+            let tail = *self.tail.get_mut();
+
+            let hix = head & (self.one_lap - 1);
+            let tix = tail & (self.one_lap - 1);
+
+            let len = if hix < tix {
+                tix - hix
+            } else if hix > tix {
+                self.cap - hix + tix
+            } else if tail == head {
+                0
             } else {
-                hix + i - self.cap
+                self.cap
             };
 
-            unsafe {
-                debug_assert!(index < self.buffer.len());
-                let slot = self.buffer.get_unchecked_mut(index);
-                (*slot.value.get()).assume_init_drop();
+            // Loop over all slots that hold a message and drop them.
+            for i in 0..len {
+                // Compute the index of the next slot holding a message.
+                let index = if hix + i < self.cap {
+                    hix + i
+                } else {
+                    hix + i - self.cap
+                };
+
+                unsafe {
+                    debug_assert!(index < self.buffer.len());
+                    let slot = self.buffer.get_unchecked_mut(index);
+                    (*slot.value.get()).assume_init_drop();
+                }
             }
         }
     }