sync: don't leak tracing spans in mutex guards (#5469)

tokio/src/sync/mutex.rs

@@ -6,9 +6,10 @@ use crate::util::trace;
 
 use std::cell::UnsafeCell;
 use std::error::Error;
+use std::marker::PhantomData;
 use std::ops::{Deref, DerefMut};
 use std::sync::Arc;
-use std::{fmt, marker, mem};
+use std::{fmt, mem};
 
 /// An asynchronous `Mutex`-like type.
 ///
@@ -144,6 +145,8 @@ pub struct Mutex<T: ?Sized> {
 #[clippy::has_significant_drop]
 #[must_use = "if unused the Mutex will immediately unlock"]
 pub struct MutexGuard<'a, T: ?Sized> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     #[cfg(all(tokio_unstable, feature = "tracing"))]
     resource_span: tracing::Span,
     lock: &'a Mutex<T>,
@@ -179,10 +182,29 @@ pub struct OwnedMutexGuard<T: ?Sized> {
 #[clippy::has_significant_drop]
 #[must_use = "if unused the Mutex will immediately unlock"]
 pub struct MappedMutexGuard<'a, T: ?Sized> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     s: &'a semaphore::Semaphore,
     data: *mut T,
     // Needed to tell the borrow checker that we are holding a `&mut T`
-    marker: marker::PhantomData<&'a mut T>,
+    marker: PhantomData<&'a mut T>,
+}
+
+/// A helper type used when taking apart a `MutexGuard` without running its
+/// Drop implementation.
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct MutexGuardInner<'a, T: ?Sized> {
+    #[cfg(all(tokio_unstable, feature = "tracing"))]
+    resource_span: tracing::Span,
+    lock: &'a Mutex<T>,
+}
+
+/// A helper type used when taking apart a `MappedMutexGuard` without running
+/// its Drop implementation.
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct MappedMutexGuardInner<'a, T: ?Sized> {
+    s: &'a semaphore::Semaphore,
+    data: *mut T,
 }
 
 // As long as T: Send, it's fine to send and share Mutex<T> between threads.
@@ -340,15 +362,27 @@ impl<T: ?Sized> Mutex<T> {
     /// }
     /// ```
     pub async fn lock(&self) -> MutexGuard<'_, T> {
+        let acquire_fut = async {
+            self.acquire().await;
+
+            MutexGuard {
+                lock: self,
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: self.resource_span.clone(),
+            }
+        };
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        trace::async_op(
-            || self.acquire(),
+        let acquire_fut = trace::async_op(
+            move || acquire_fut,
             self.resource_span.clone(),
             "Mutex::lock",
             "poll",
             false,
-        )
-        .await;
+        );
+
+        #[allow(clippy::let_and_return)] // this lint triggers when disabling tracing
+        let guard = acquire_fut.await;
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {
@@ -358,14 +392,7 @@ impl<T: ?Sized> Mutex<T> {
             );
         });
 
-        #[cfg(any(not(tokio_unstable), not(feature = "tracing")))]
-        self.acquire().await;
-
-        MutexGuard {
-            lock: self,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span: self.resource_span.clone(),
-        }
+        guard
     }
 
     /// Blockingly locks this `Mutex`. When the lock has been acquired, function returns a
@@ -512,34 +539,39 @@ impl<T: ?Sized> Mutex<T> {
     /// [`Arc`]: std::sync::Arc
     pub async fn lock_owned(self: Arc<Self>) -> OwnedMutexGuard<T> {
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        trace::async_op(
-            || self.acquire(),
-            self.resource_span.clone(),
+        let resource_span = self.resource_span.clone();
+
+        let acquire_fut = async {
+            self.acquire().await;
+
+            OwnedMutexGuard {
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: self.resource_span.clone(),
+                lock: self,
+            }
+        };
+
+        #[cfg(all(tokio_unstable, feature = "tracing"))]
+        let acquire_fut = trace::async_op(
+            move || acquire_fut,
+            resource_span,
             "Mutex::lock_owned",
             "poll",
             false,
-        )
-        .await;
+        );
+
+        #[allow(clippy::let_and_return)] // this lint triggers when disabling tracing
+        let guard = acquire_fut.await;
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
                 target: "runtime::resource::state_update",
                 locked = true,
             );
         });
 
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = self.resource_span.clone();
-
-        #[cfg(any(not(tokio_unstable), not(feature = "tracing")))]
-        self.acquire().await;
-
-        OwnedMutexGuard {
-            lock: self,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
-        }
+        guard
     }
 
     async fn acquire(&self) {
@@ -570,6 +602,12 @@ impl<T: ?Sized> Mutex<T> {
     pub fn try_lock(&self) -> Result<MutexGuard<'_, T>, TryLockError> {
         match self.s.try_acquire(1) {
             Ok(_) => {
+                let guard = MutexGuard {
+                    lock: self,
+                    #[cfg(all(tokio_unstable, feature = "tracing"))]
+                    resource_span: self.resource_span.clone(),
+                };
+
                 #[cfg(all(tokio_unstable, feature = "tracing"))]
                 self.resource_span.in_scope(|| {
                     tracing::trace!(
@@ -578,11 +616,7 @@ impl<T: ?Sized> Mutex<T> {
                     );
                 });
 
-                Ok(MutexGuard {
-                    lock: self,
-                    #[cfg(all(tokio_unstable, feature = "tracing"))]
-                    resource_span: self.resource_span.clone(),
-                })
+                Ok(guard)
             }
             Err(_) => Err(TryLockError(())),
         }
@@ -639,22 +673,21 @@ impl<T: ?Sized> Mutex<T> {
     pub fn try_lock_owned(self: Arc<Self>) -> Result<OwnedMutexGuard<T>, TryLockError> {
         match self.s.try_acquire(1) {
             Ok(_) => {
+                let guard = OwnedMutexGuard {
+                    #[cfg(all(tokio_unstable, feature = "tracing"))]
+                    resource_span: self.resource_span.clone(),
+                    lock: self,
+                };
+
                 #[cfg(all(tokio_unstable, feature = "tracing"))]
-                self.resource_span.in_scope(|| {
+                guard.resource_span.in_scope(|| {
                     tracing::trace!(
                         target: "runtime::resource::state_update",
                         locked = true,
                     );
                 });
 
-                #[cfg(all(tokio_unstable, feature = "tracing"))]
-                let resource_span = self.resource_span.clone();
-
-                Ok(OwnedMutexGuard {
-                    lock: self,
-                    #[cfg(all(tokio_unstable, feature = "tracing"))]
-                    resource_span,
-                })
+                Ok(guard)
             }
             Err(_) => Err(TryLockError(())),
         }
@@ -714,6 +747,17 @@ where
 // === impl MutexGuard ===
 
 impl<'a, T: ?Sized> MutexGuard<'a, T> {
+    fn skip_drop(self) -> MutexGuardInner<'a, T> {
+        let me = mem::ManuallyDrop::new(self);
+        // SAFETY: This duplicates the `resource_span` and then forgets the
+        // original. In the end, we have not duplicated or forgotten any values.
+        MutexGuardInner {
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: unsafe { std::ptr::read(&me.resource_span) },
+            lock: me.lock,
+        }
+    }
+
     /// Makes a new [`MappedMutexGuard`] for a component of the locked data.
     ///
     /// This operation cannot fail as the [`MutexGuard`] passed in already locked the mutex.
@@ -750,12 +794,11 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> {
         F: FnOnce(&mut T) -> &mut U,
     {
         let data = f(&mut *this) as *mut U;
-        let s = &this.lock.s;
-        mem::forget(this);
+        let inner = this.skip_drop();
         MappedMutexGuard {
-            s,
+            s: &inner.lock.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
         }
     }
 
@@ -800,12 +843,11 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> {
             Some(data) => data as *mut U,
             None => return Err(this),
         };
-        let s = &this.lock.s;
-        mem::forget(this);
+        let inner = this.skip_drop();
         Ok(MappedMutexGuard {
-            s,
+            s: &inner.lock.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
         })
     }
 
@@ -946,6 +988,14 @@ impl<T: ?Sized + fmt::Display> fmt::Display for OwnedMutexGuard<T> {
 // === impl MappedMutexGuard ===
 
 impl<'a, T: ?Sized> MappedMutexGuard<'a, T> {
+    fn skip_drop(self) -> MappedMutexGuardInner<'a, T> {
+        let me = mem::ManuallyDrop::new(self);
+        MappedMutexGuardInner {
+            s: me.s,
+            data: me.data,
+        }
+    }
+
     /// Makes a new [`MappedMutexGuard`] for a component of the locked data.
     ///
     /// This operation cannot fail as the [`MappedMutexGuard`] passed in already locked the mutex.
@@ -960,12 +1010,11 @@ impl<'a, T: ?Sized> MappedMutexGuard<'a, T> {
         F: FnOnce(&mut T) -> &mut U,
     {
         let data = f(&mut *this) as *mut U;
-        let s = this.s;
-        mem::forget(this);
+        let inner = this.skip_drop();
         MappedMutexGuard {
-            s,
+            s: inner.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
         }
     }
 
@@ -987,12 +1036,11 @@ impl<'a, T: ?Sized> MappedMutexGuard<'a, T> {
             Some(data) => data as *mut U,
             None => return Err(this),
         };
-        let s = this.s;
-        mem::forget(this);
+        let inner = this.skip_drop();
         Ok(MappedMutexGuard {
-            s,
+            s: inner.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
         })
     }
 }

tokio/src/sync/rwlock.rs

@@ -5,7 +5,6 @@ use crate::util::trace;
 use std::cell::UnsafeCell;
 use std::marker;
 use std::marker::PhantomData;
-use std::mem::ManuallyDrop;
 use std::sync::Arc;
 
 pub(crate) mod owned_read_guard;
@@ -423,23 +422,33 @@ impl<T: ?Sized> RwLock<T> {
     /// }
     /// ```
     pub async fn read(&self) -> RwLockReadGuard<'_, T> {
+        let acquire_fut = async {
+            self.s.acquire(1).await.unwrap_or_else(|_| {
+                // The semaphore was closed. but, we never explicitly close it, and we have a
+                // handle to it through the Arc, which means that this can never happen.
+                unreachable!()
+            });
+
+            RwLockReadGuard {
+                s: &self.s,
+                data: self.c.get(),
+                marker: PhantomData,
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: self.resource_span.clone(),
+            }
+        };
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let inner = trace::async_op(
-            || self.s.acquire(1),
+        let acquire_fut = trace::async_op(
+            move || acquire_fut,
             self.resource_span.clone(),
             "RwLock::read",
             "poll",
             false,
         );
 
-        #[cfg(not(all(tokio_unstable, feature = "tracing")))]
-        let inner = self.s.acquire(1);
-
-        inner.await.unwrap_or_else(|_| {
-            // The semaphore was closed. but, we never explicitly close it, and we have a
-            // handle to it through the Arc, which means that this can never happen.
-            unreachable!()
-        });
+        #[allow(clippy::let_and_return)] // this lint triggers when disabling tracing
+        let guard = acquire_fut.await;
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {
@@ -450,13 +459,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        RwLockReadGuard {
-            s: &self.s,
-            data: self.c.get(),
-            marker: marker::PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span: self.resource_span.clone(),
-        }
+        guard
     }
 
     /// Blockingly locks this `RwLock` with shared read access.
@@ -565,25 +568,38 @@ impl<T: ?Sized> RwLock<T> {
     /// ```
     pub async fn read_owned(self: Arc<Self>) -> OwnedRwLockReadGuard<T> {
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let inner = trace::async_op(
-            || self.s.acquire(1),
-            self.resource_span.clone(),
+        let resource_span = self.resource_span.clone();
+
+        let acquire_fut = async {
+            self.s.acquire(1).await.unwrap_or_else(|_| {
+                // The semaphore was closed. but, we never explicitly close it, and we have a
+                // handle to it through the Arc, which means that this can never happen.
+                unreachable!()
+            });
+
+            OwnedRwLockReadGuard {
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: self.resource_span.clone(),
+                data: self.c.get(),
+                lock: self,
+                _p: PhantomData,
+            }
+        };
+
+        #[cfg(all(tokio_unstable, feature = "tracing"))]
+        let acquire_fut = trace::async_op(
+            move || acquire_fut,
+            resource_span,
             "RwLock::read_owned",
             "poll",
             false,
         );
 
-        #[cfg(not(all(tokio_unstable, feature = "tracing")))]
-        let inner = self.s.acquire(1);
-
-        inner.await.unwrap_or_else(|_| {
-            // The semaphore was closed. but, we never explicitly close it, and we have a
-            // handle to it through the Arc, which means that this can never happen.
-            unreachable!()
-        });
+        #[allow(clippy::let_and_return)] // this lint triggers when disabling tracing
+        let guard = acquire_fut.await;
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             current_readers = 1,
@@ -591,16 +607,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = self.resource_span.clone();
-
-        OwnedRwLockReadGuard {
-            data: self.c.get(),
-            lock: ManuallyDrop::new(self),
-            _p: PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
-        }
+        guard
     }
 
     /// Attempts to acquire this `RwLock` with shared read access.
@@ -642,6 +649,14 @@ impl<T: ?Sized> RwLock<T> {
             Err(TryAcquireError::Closed) => unreachable!(),
         }
 
+        let guard = RwLockReadGuard {
+            s: &self.s,
+            data: self.c.get(),
+            marker: marker::PhantomData,
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: self.resource_span.clone(),
+        };
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {
             tracing::trace!(
@@ -651,13 +666,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        Ok(RwLockReadGuard {
-            s: &self.s,
-            data: self.c.get(),
-            marker: marker::PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span: self.resource_span.clone(),
-        })
+        Ok(guard)
     }
 
     /// Attempts to acquire this `RwLock` with shared read access.
@@ -705,8 +714,16 @@ impl<T: ?Sized> RwLock<T> {
             Err(TryAcquireError::Closed) => unreachable!(),
         }
 
+        let guard = OwnedRwLockReadGuard {
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: self.resource_span.clone(),
+            data: self.c.get(),
+            lock: self,
+            _p: PhantomData,
+        };
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             current_readers = 1,
@@ -714,16 +731,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = self.resource_span.clone();
-
-        Ok(OwnedRwLockReadGuard {
-            data: self.c.get(),
-            lock: ManuallyDrop::new(self),
-            _p: PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
-        })
+        Ok(guard)
     }
 
     /// Locks this `RwLock` with exclusive write access, causing the current
@@ -755,23 +763,34 @@ impl<T: ?Sized> RwLock<T> {
     ///}
     /// ```
     pub async fn write(&self) -> RwLockWriteGuard<'_, T> {
+        let acquire_fut = async {
+            self.s.acquire(self.mr).await.unwrap_or_else(|_| {
+                // The semaphore was closed. but, we never explicitly close it, and we have a
+                // handle to it through the Arc, which means that this can never happen.
+                unreachable!()
+            });
+
+            RwLockWriteGuard {
+                permits_acquired: self.mr,
+                s: &self.s,
+                data: self.c.get(),
+                marker: marker::PhantomData,
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: self.resource_span.clone(),
+            }
+        };
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let inner = trace::async_op(
-            || self.s.acquire(self.mr),
+        let acquire_fut = trace::async_op(
+            move || acquire_fut,
             self.resource_span.clone(),
             "RwLock::write",
             "poll",
             false,
         );
 
-        #[cfg(not(all(tokio_unstable, feature = "tracing")))]
-        let inner = self.s.acquire(self.mr);
-
-        inner.await.unwrap_or_else(|_| {
-            // The semaphore was closed. but, we never explicitly close it, and we have a
-            // handle to it through the Arc, which means that this can never happen.
-            unreachable!()
-        });
+        #[allow(clippy::let_and_return)] // this lint triggers when disabling tracing
+        let guard = acquire_fut.await;
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {
@@ -782,14 +801,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        RwLockWriteGuard {
-            permits_acquired: self.mr,
-            s: &self.s,
-            data: self.c.get(),
-            marker: marker::PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span: self.resource_span.clone(),
-        }
+        guard
     }
 
     /// Blockingly locks this `RwLock` with exclusive write access.
@@ -884,25 +896,39 @@ impl<T: ?Sized> RwLock<T> {
     /// ```
     pub async fn write_owned(self: Arc<Self>) -> OwnedRwLockWriteGuard<T> {
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let inner = trace::async_op(
-            || self.s.acquire(self.mr),
-            self.resource_span.clone(),
+        let resource_span = self.resource_span.clone();
+
+        let acquire_fut = async {
+            self.s.acquire(self.mr).await.unwrap_or_else(|_| {
+                // The semaphore was closed. but, we never explicitly close it, and we have a
+                // handle to it through the Arc, which means that this can never happen.
+                unreachable!()
+            });
+
+            OwnedRwLockWriteGuard {
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: self.resource_span.clone(),
+                permits_acquired: self.mr,
+                data: self.c.get(),
+                lock: self,
+                _p: PhantomData,
+            }
+        };
+
+        #[cfg(all(tokio_unstable, feature = "tracing"))]
+        let acquire_fut = trace::async_op(
+            move || acquire_fut,
+            resource_span,
             "RwLock::write_owned",
             "poll",
             false,
         );
 
-        #[cfg(not(all(tokio_unstable, feature = "tracing")))]
-        let inner = self.s.acquire(self.mr);
-
-        inner.await.unwrap_or_else(|_| {
-            // The semaphore was closed. but, we never explicitly close it, and we have a
-            // handle to it through the Arc, which means that this can never happen.
-            unreachable!()
-        });
+        #[allow(clippy::let_and_return)] // this lint triggers when disabling tracing
+        let guard = acquire_fut.await;
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             write_locked = true,
@@ -910,17 +936,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = self.resource_span.clone();
-
-        OwnedRwLockWriteGuard {
-            permits_acquired: self.mr,
-            data: self.c.get(),
-            lock: ManuallyDrop::new(self),
-            _p: PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
-        }
+        guard
     }
 
     /// Attempts to acquire this `RwLock` with exclusive write access.
@@ -953,6 +969,15 @@ impl<T: ?Sized> RwLock<T> {
             Err(TryAcquireError::Closed) => unreachable!(),
         }
 
+        let guard = RwLockWriteGuard {
+            permits_acquired: self.mr,
+            s: &self.s,
+            data: self.c.get(),
+            marker: marker::PhantomData,
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: self.resource_span.clone(),
+        };
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {
             tracing::trace!(
@@ -962,14 +987,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        Ok(RwLockWriteGuard {
-            permits_acquired: self.mr,
-            s: &self.s,
-            data: self.c.get(),
-            marker: marker::PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span: self.resource_span.clone(),
-        })
+        Ok(guard)
     }
 
     /// Attempts to acquire this `RwLock` with exclusive write access.
@@ -1009,8 +1027,17 @@ impl<T: ?Sized> RwLock<T> {
             Err(TryAcquireError::Closed) => unreachable!(),
         }
 
+        let guard = OwnedRwLockWriteGuard {
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: self.resource_span.clone(),
+            permits_acquired: self.mr,
+            data: self.c.get(),
+            lock: self,
+            _p: PhantomData,
+        };
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             write_locked = true,
@@ -1018,17 +1045,7 @@ impl<T: ?Sized> RwLock<T> {
             )
         });
 
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = self.resource_span.clone();
-
-        Ok(OwnedRwLockWriteGuard {
-            permits_acquired: self.mr,
-            data: self.c.get(),
-            lock: ManuallyDrop::new(self),
-            _p: PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
-        })
+        Ok(guard)
     }
 
     /// Returns a mutable reference to the underlying data.

tokio/src/sync/rwlock/owned_read_guard.rs

@@ -1,10 +1,7 @@
 use crate::sync::rwlock::RwLock;
-use std::fmt;
 use std::marker::PhantomData;
-use std::mem;
-use std::mem::ManuallyDrop;
-use std::ops;
 use std::sync::Arc;
+use std::{fmt, mem, ops, ptr};
 
 /// Owned RAII structure used to release the shared read access of a lock when
 /// dropped.
@@ -16,15 +13,38 @@ use std::sync::Arc;
 /// [`RwLock`]: struct@crate::sync::RwLock
 #[clippy::has_significant_drop]
 pub struct OwnedRwLockReadGuard<T: ?Sized, U: ?Sized = T> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     #[cfg(all(tokio_unstable, feature = "tracing"))]
     pub(super) resource_span: tracing::Span,
-    // ManuallyDrop allows us to destructure into this field without running the destructor.
-    pub(super) lock: ManuallyDrop<Arc<RwLock<T>>>,
+    pub(super) lock: Arc<RwLock<T>>,
     pub(super) data: *const U,
     pub(super) _p: PhantomData<T>,
 }
 
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct Inner<T: ?Sized, U: ?Sized> {
+    #[cfg(all(tokio_unstable, feature = "tracing"))]
+    resource_span: tracing::Span,
+    lock: Arc<RwLock<T>>,
+    data: *const U,
+}
+
 impl<T: ?Sized, U: ?Sized> OwnedRwLockReadGuard<T, U> {
+    fn skip_drop(self) -> Inner<T, U> {
+        let me = mem::ManuallyDrop::new(self);
+        // SAFETY: This duplicates the values in every field of the guard, then
+        // forgets the originals, so in the end no value is duplicated.
+        unsafe {
+            Inner {
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: ptr::read(&me.resource_span),
+                lock: ptr::read(&me.lock),
+                data: me.data,
+            }
+        }
+    }
+
     /// Makes a new `OwnedRwLockReadGuard` for a component of the locked data.
     /// This operation cannot fail as the `OwnedRwLockReadGuard` passed in
     /// already locked the data.
@@ -53,23 +73,19 @@ impl<T: ?Sized, U: ?Sized> OwnedRwLockReadGuard<T, U> {
     /// # }
     /// ```
     #[inline]
-    pub fn map<F, V: ?Sized>(mut this: Self, f: F) -> OwnedRwLockReadGuard<T, V>
+    pub fn map<F, V: ?Sized>(this: Self, f: F) -> OwnedRwLockReadGuard<T, V>
     where
         F: FnOnce(&U) -> &V,
     {
         let data = f(&*this) as *const V;
-        let lock = unsafe { ManuallyDrop::take(&mut this.lock) };
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         OwnedRwLockReadGuard {
-            lock: ManuallyDrop::new(lock),
+            lock: this.lock,
             data,
             _p: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         }
     }
 
@@ -104,7 +120,7 @@ impl<T: ?Sized, U: ?Sized> OwnedRwLockReadGuard<T, U> {
     /// # }
     /// ```
     #[inline]
-    pub fn try_map<F, V: ?Sized>(mut this: Self, f: F) -> Result<OwnedRwLockReadGuard<T, V>, Self>
+    pub fn try_map<F, V: ?Sized>(this: Self, f: F) -> Result<OwnedRwLockReadGuard<T, V>, Self>
     where
         F: FnOnce(&U) -> Option<&V>,
     {
@@ -112,18 +128,14 @@ impl<T: ?Sized, U: ?Sized> OwnedRwLockReadGuard<T, U> {
             Some(data) => data as *const V,
             None => return Err(this),
         };
-        let lock = unsafe { ManuallyDrop::take(&mut this.lock) };
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         Ok(OwnedRwLockReadGuard {
-            lock: ManuallyDrop::new(lock),
+            lock: this.lock,
             data,
             _p: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         })
     }
 }
@@ -157,7 +169,6 @@ where
 impl<T: ?Sized, U: ?Sized> Drop for OwnedRwLockReadGuard<T, U> {
     fn drop(&mut self) {
         self.lock.s.release(1);
-        unsafe { ManuallyDrop::drop(&mut self.lock) };
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {

tokio/src/sync/rwlock/owned_write_guard.rs

@@ -1,11 +1,9 @@
 use crate::sync::rwlock::owned_read_guard::OwnedRwLockReadGuard;
 use crate::sync::rwlock::owned_write_guard_mapped::OwnedRwLockMappedWriteGuard;
 use crate::sync::rwlock::RwLock;
-use std::fmt;
 use std::marker::PhantomData;
-use std::mem::{self, ManuallyDrop};
-use std::ops;
 use std::sync::Arc;
+use std::{fmt, mem, ops, ptr};
 
 /// Owned RAII structure used to release the exclusive write access of a lock when
 /// dropped.
@@ -17,16 +15,41 @@ use std::sync::Arc;
 /// [`RwLock`]: struct@crate::sync::RwLock
 #[clippy::has_significant_drop]
 pub struct OwnedRwLockWriteGuard<T: ?Sized> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     #[cfg(all(tokio_unstable, feature = "tracing"))]
     pub(super) resource_span: tracing::Span,
     pub(super) permits_acquired: u32,
-    // ManuallyDrop allows us to destructure into this field without running the destructor.
-    pub(super) lock: ManuallyDrop<Arc<RwLock<T>>>,
+    pub(super) lock: Arc<RwLock<T>>,
     pub(super) data: *mut T,
     pub(super) _p: PhantomData<T>,
 }
 
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct Inner<T: ?Sized> {
+    #[cfg(all(tokio_unstable, feature = "tracing"))]
+    resource_span: tracing::Span,
+    permits_acquired: u32,
+    lock: Arc<RwLock<T>>,
+    data: *const T,
+}
+
 impl<T: ?Sized> OwnedRwLockWriteGuard<T> {
+    fn skip_drop(self) -> Inner<T> {
+        let me = mem::ManuallyDrop::new(self);
+        // SAFETY: This duplicates the values in every field of the guard, then
+        // forgets the originals, so in the end no value is duplicated.
+        unsafe {
+            Inner {
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: ptr::read(&me.resource_span),
+                permits_acquired: me.permits_acquired,
+                lock: ptr::read(&me.lock),
+                data: me.data,
+            }
+        }
+    }
+
     /// Makes a new [`OwnedRwLockMappedWriteGuard`] for a component of the locked
     /// data.
     ///
@@ -65,20 +88,15 @@ impl<T: ?Sized> OwnedRwLockWriteGuard<T> {
         F: FnOnce(&mut T) -> &mut U,
     {
         let data = f(&mut *this) as *mut U;
-        let lock = unsafe { ManuallyDrop::take(&mut this.lock) };
-        let permits_acquired = this.permits_acquired;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         OwnedRwLockMappedWriteGuard {
-            permits_acquired,
-            lock: ManuallyDrop::new(lock),
+            permits_acquired: this.permits_acquired,
+            lock: this.lock,
             data,
             _p: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         }
     }
 
@@ -129,21 +147,15 @@ impl<T: ?Sized> OwnedRwLockWriteGuard<T> {
             Some(data) => data as *mut U,
             None => return Err(this),
         };
-        let permits_acquired = this.permits_acquired;
-        let lock = unsafe { ManuallyDrop::take(&mut this.lock) };
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         Ok(OwnedRwLockMappedWriteGuard {
-            permits_acquired,
-            lock: ManuallyDrop::new(lock),
+            permits_acquired: this.permits_acquired,
+            lock: this.lock,
             data,
             _p: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         })
     }
 
@@ -192,15 +204,22 @@ impl<T: ?Sized> OwnedRwLockWriteGuard<T> {
     /// assert_eq!(*lock.read().await, 2, "second writer obtained write lock");
     /// # }
     /// ```
-    pub fn downgrade(mut self) -> OwnedRwLockReadGuard<T> {
-        let lock = unsafe { ManuallyDrop::take(&mut self.lock) };
-        let data = self.data;
-        let to_release = (self.permits_acquired - 1) as usize;
+    pub fn downgrade(self) -> OwnedRwLockReadGuard<T> {
+        let this = self.skip_drop();
+        let guard = OwnedRwLockReadGuard {
+            lock: this.lock,
+            data: this.data,
+            _p: PhantomData,
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: this.resource_span,
+        };
 
         // Release all but one of the permits held by the write guard
-        lock.s.release(to_release);
+        let to_release = (this.permits_acquired - 1) as usize;
+        guard.lock.s.release(to_release);
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             write_locked = false,
@@ -209,7 +228,7 @@ impl<T: ?Sized> OwnedRwLockWriteGuard<T> {
         });
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             current_readers = 1,
@@ -217,18 +236,7 @@ impl<T: ?Sized> OwnedRwLockWriteGuard<T> {
             )
         });
 
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = self.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(self);
-
-        OwnedRwLockReadGuard {
-            lock: ManuallyDrop::new(lock),
-            data,
-            _p: PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
-        }
+        guard
     }
 }
 
@@ -267,6 +275,7 @@ where
 impl<T: ?Sized> Drop for OwnedRwLockWriteGuard<T> {
     fn drop(&mut self) {
         self.lock.s.release(self.permits_acquired as usize);
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {
             tracing::trace!(
@@ -275,6 +284,5 @@ impl<T: ?Sized> Drop for OwnedRwLockWriteGuard<T> {
             write_locked.op = "override",
             )
         });
-        unsafe { ManuallyDrop::drop(&mut self.lock) };
     }
 }

tokio/src/sync/rwlock/owned_write_guard_mapped.rs

@@ -1,9 +1,7 @@
 use crate::sync::rwlock::RwLock;
-use std::fmt;
 use std::marker::PhantomData;
-use std::mem::{self, ManuallyDrop};
-use std::ops;
 use std::sync::Arc;
+use std::{fmt, mem, ops, ptr};
 
 /// Owned RAII structure used to release the exclusive write access of a lock when
 /// dropped.
@@ -16,16 +14,41 @@ use std::sync::Arc;
 /// [`OwnedRwLockWriteGuard`]: struct@crate::sync::OwnedRwLockWriteGuard
 #[clippy::has_significant_drop]
 pub struct OwnedRwLockMappedWriteGuard<T: ?Sized, U: ?Sized = T> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     #[cfg(all(tokio_unstable, feature = "tracing"))]
     pub(super) resource_span: tracing::Span,
     pub(super) permits_acquired: u32,
-    // ManuallyDrop allows us to destructure into this field without running the destructor.
-    pub(super) lock: ManuallyDrop<Arc<RwLock<T>>>,
+    pub(super) lock: Arc<RwLock<T>>,
     pub(super) data: *mut U,
     pub(super) _p: PhantomData<T>,
 }
 
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct Inner<T: ?Sized, U: ?Sized> {
+    #[cfg(all(tokio_unstable, feature = "tracing"))]
+    resource_span: tracing::Span,
+    permits_acquired: u32,
+    lock: Arc<RwLock<T>>,
+    data: *const U,
+}
+
 impl<T: ?Sized, U: ?Sized> OwnedRwLockMappedWriteGuard<T, U> {
+    fn skip_drop(self) -> Inner<T, U> {
+        let me = mem::ManuallyDrop::new(self);
+        // SAFETY: This duplicates the values in every field of the guard, then
+        // forgets the originals, so in the end no value is duplicated.
+        unsafe {
+            Inner {
+                #[cfg(all(tokio_unstable, feature = "tracing"))]
+                resource_span: ptr::read(&me.resource_span),
+                permits_acquired: me.permits_acquired,
+                lock: ptr::read(&me.lock),
+                data: me.data,
+            }
+        }
+    }
+
     /// Makes a new `OwnedRwLockMappedWriteGuard` for a component of the locked
     /// data.
     ///
@@ -64,20 +87,15 @@ impl<T: ?Sized, U: ?Sized> OwnedRwLockMappedWriteGuard<T, U> {
         F: FnOnce(&mut U) -> &mut V,
     {
         let data = f(&mut *this) as *mut V;
-        let lock = unsafe { ManuallyDrop::take(&mut this.lock) };
-        let permits_acquired = this.permits_acquired;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         OwnedRwLockMappedWriteGuard {
-            permits_acquired,
-            lock: ManuallyDrop::new(lock),
+            permits_acquired: this.permits_acquired,
+            lock: this.lock,
             data,
             _p: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         }
     }
 
@@ -126,20 +144,15 @@ impl<T: ?Sized, U: ?Sized> OwnedRwLockMappedWriteGuard<T, U> {
             Some(data) => data as *mut V,
             None => return Err(this),
         };
-        let lock = unsafe { ManuallyDrop::take(&mut this.lock) };
-        let permits_acquired = this.permits_acquired;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         Ok(OwnedRwLockMappedWriteGuard {
-            permits_acquired,
-            lock: ManuallyDrop::new(lock),
+            permits_acquired: this.permits_acquired,
+            lock: this.lock,
             data,
             _p: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         })
     }
 }
@@ -179,6 +192,7 @@ where
 impl<T: ?Sized, U: ?Sized> Drop for OwnedRwLockMappedWriteGuard<T, U> {
     fn drop(&mut self) {
         self.lock.s.release(self.permits_acquired as usize);
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
         self.resource_span.in_scope(|| {
             tracing::trace!(
@@ -187,6 +201,5 @@ impl<T: ?Sized, U: ?Sized> Drop for OwnedRwLockMappedWriteGuard<T, U> {
             write_locked.op = "override",
             )
         });
-        unsafe { ManuallyDrop::drop(&mut self.lock) };
     }
 }

tokio/src/sync/rwlock/read_guard.rs

@@ -1,8 +1,6 @@
 use crate::sync::batch_semaphore::Semaphore;
-use std::fmt;
-use std::marker;
-use std::mem;
-use std::ops;
+use std::marker::PhantomData;
+use std::{fmt, mem, ops};
 
 /// RAII structure used to release the shared read access of a lock when
 /// dropped.
@@ -15,14 +13,36 @@ use std::ops;
 #[clippy::has_significant_drop]
 #[must_use = "if unused the RwLock will immediately unlock"]
 pub struct RwLockReadGuard<'a, T: ?Sized> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     #[cfg(all(tokio_unstable, feature = "tracing"))]
     pub(super) resource_span: tracing::Span,
     pub(super) s: &'a Semaphore,
     pub(super) data: *const T,
-    pub(super) marker: marker::PhantomData<&'a T>,
+    pub(super) marker: PhantomData<&'a T>,
+}
+
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct Inner<'a, T: ?Sized> {
+    #[cfg(all(tokio_unstable, feature = "tracing"))]
+    resource_span: tracing::Span,
+    s: &'a Semaphore,
+    data: *const T,
 }
 
 impl<'a, T: ?Sized> RwLockReadGuard<'a, T> {
+    fn skip_drop(self) -> Inner<'a, T> {
+        let me = mem::ManuallyDrop::new(self);
+        // SAFETY: This duplicates the values in every field of the guard, then
+        // forgets the originals, so in the end no value is duplicated.
+        Inner {
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: unsafe { std::ptr::read(&me.resource_span) },
+            s: me.s,
+            data: me.data,
+        }
+    }
+
     /// Makes a new `RwLockReadGuard` for a component of the locked data.
     ///
     /// This operation cannot fail as the `RwLockReadGuard` passed in already
@@ -62,18 +82,14 @@ impl<'a, T: ?Sized> RwLockReadGuard<'a, T> {
         F: FnOnce(&T) -> &U,
     {
         let data = f(&*this) as *const U;
-        let s = this.s;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         RwLockReadGuard {
-            s,
+            s: this.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         }
     }
 
@@ -121,18 +137,14 @@ impl<'a, T: ?Sized> RwLockReadGuard<'a, T> {
             Some(data) => data as *const U,
             None => return Err(this),
         };
-        let s = this.s;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         Ok(RwLockReadGuard {
-            s,
+            s: this.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         })
     }
 }

tokio/src/sync/rwlock/write_guard.rs

@@ -1,10 +1,8 @@
 use crate::sync::batch_semaphore::Semaphore;
 use crate::sync::rwlock::read_guard::RwLockReadGuard;
 use crate::sync::rwlock::write_guard_mapped::RwLockMappedWriteGuard;
-use std::fmt;
-use std::marker;
-use std::mem;
-use std::ops;
+use std::marker::PhantomData;
+use std::{fmt, mem, ops};
 
 /// RAII structure used to release the exclusive write access of a lock when
 /// dropped.
@@ -17,15 +15,39 @@ use std::ops;
 #[clippy::has_significant_drop]
 #[must_use = "if unused the RwLock will immediately unlock"]
 pub struct RwLockWriteGuard<'a, T: ?Sized> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     #[cfg(all(tokio_unstable, feature = "tracing"))]
     pub(super) resource_span: tracing::Span,
     pub(super) permits_acquired: u32,
     pub(super) s: &'a Semaphore,
     pub(super) data: *mut T,
-    pub(super) marker: marker::PhantomData<&'a mut T>,
+    pub(super) marker: PhantomData<&'a mut T>,
+}
+
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct Inner<'a, T: ?Sized> {
+    #[cfg(all(tokio_unstable, feature = "tracing"))]
+    resource_span: tracing::Span,
+    permits_acquired: u32,
+    s: &'a Semaphore,
+    data: *mut T,
 }
 
 impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
+    fn skip_drop(self) -> Inner<'a, T> {
+        let me = mem::ManuallyDrop::new(self);
+        // SAFETY: This duplicates the values in every field of the guard, then
+        // forgets the originals, so in the end no value is duplicated.
+        Inner {
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: unsafe { std::ptr::read(&me.resource_span) },
+            permits_acquired: me.permits_acquired,
+            s: me.s,
+            data: me.data,
+        }
+    }
+
     /// Makes a new [`RwLockMappedWriteGuard`] for a component of the locked data.
     ///
     /// This operation cannot fail as the `RwLockWriteGuard` passed in already
@@ -68,19 +90,15 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
         F: FnOnce(&mut T) -> &mut U,
     {
         let data = f(&mut *this) as *mut U;
-        let s = this.s;
-        let permits_acquired = this.permits_acquired;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
+
         RwLockMappedWriteGuard {
-            permits_acquired,
-            s,
+            permits_acquired: this.permits_acquired,
+            s: this.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         }
     }
 
@@ -135,19 +153,15 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
             Some(data) => data as *mut U,
             None => return Err(this),
         };
-        let s = this.s;
-        let permits_acquired = this.permits_acquired;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
+
         Ok(RwLockMappedWriteGuard {
-            permits_acquired,
-            s,
+            permits_acquired: this.permits_acquired,
+            s: this.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         })
     }
 
@@ -199,12 +213,21 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
     ///
     /// [`RwLock`]: struct@crate::sync::RwLock
     pub fn downgrade(self) -> RwLockReadGuard<'a, T> {
-        let RwLockWriteGuard { s, data, .. } = self;
-        let to_release = (self.permits_acquired - 1) as usize;
+        let this = self.skip_drop();
+        let guard = RwLockReadGuard {
+            s: this.s,
+            data: this.data,
+            marker: PhantomData,
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: this.resource_span,
+        };
+
         // Release all but one of the permits held by the write guard
-        s.release(to_release);
+        let to_release = (this.permits_acquired - 1) as usize;
+        this.s.release(to_release);
+
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             write_locked = false,
@@ -213,7 +236,7 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
         });
 
         #[cfg(all(tokio_unstable, feature = "tracing"))]
-        self.resource_span.in_scope(|| {
+        guard.resource_span.in_scope(|| {
             tracing::trace!(
             target: "runtime::resource::state_update",
             current_readers = 1,
@@ -221,18 +244,7 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
             )
         });
 
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = self.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(self);
-
-        RwLockReadGuard {
-            s,
-            data,
-            marker: marker::PhantomData,
-            #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
-        }
+        guard
     }
 }
 

tokio/src/sync/rwlock/write_guard_mapped.rs

@@ -1,8 +1,6 @@
 use crate::sync::batch_semaphore::Semaphore;
-use std::fmt;
-use std::marker;
-use std::mem;
-use std::ops;
+use std::marker::PhantomData;
+use std::{fmt, mem, ops};
 
 /// RAII structure used to release the exclusive write access of a lock when
 /// dropped.
@@ -15,15 +13,39 @@ use std::ops;
 /// [`RwLockWriteGuard`]: struct@crate::sync::RwLockWriteGuard
 #[clippy::has_significant_drop]
 pub struct RwLockMappedWriteGuard<'a, T: ?Sized> {
+    // When changing the fields in this struct, make sure to update the
+    // `skip_drop` method.
     #[cfg(all(tokio_unstable, feature = "tracing"))]
     pub(super) resource_span: tracing::Span,
     pub(super) permits_acquired: u32,
     pub(super) s: &'a Semaphore,
     pub(super) data: *mut T,
-    pub(super) marker: marker::PhantomData<&'a mut T>,
+    pub(super) marker: PhantomData<&'a mut T>,
+}
+
+#[allow(dead_code)] // Unused fields are still used in Drop.
+struct Inner<'a, T: ?Sized> {
+    #[cfg(all(tokio_unstable, feature = "tracing"))]
+    resource_span: tracing::Span,
+    permits_acquired: u32,
+    s: &'a Semaphore,
+    data: *mut T,
 }
 
 impl<'a, T: ?Sized> RwLockMappedWriteGuard<'a, T> {
+    fn skip_drop(self) -> Inner<'a, T> {
+        let me = mem::ManuallyDrop::new(self);
+        // SAFETY: This duplicates the values in every field of the guard, then
+        // forgets the originals, so in the end no value is duplicated.
+        Inner {
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: unsafe { std::ptr::read(&me.resource_span) },
+            permits_acquired: me.permits_acquired,
+            s: me.s,
+            data: me.data,
+        }
+    }
+
     /// Makes a new `RwLockMappedWriteGuard` for a component of the locked data.
     ///
     /// This operation cannot fail as the `RwLockMappedWriteGuard` passed in already
@@ -65,20 +87,15 @@ impl<'a, T: ?Sized> RwLockMappedWriteGuard<'a, T> {
         F: FnOnce(&mut T) -> &mut U,
     {
         let data = f(&mut *this) as *mut U;
-        let s = this.s;
-        let permits_acquired = this.permits_acquired;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         RwLockMappedWriteGuard {
-            permits_acquired,
-            s,
+            permits_acquired: this.permits_acquired,
+            s: this.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         }
     }
 
@@ -132,20 +149,15 @@ impl<'a, T: ?Sized> RwLockMappedWriteGuard<'a, T> {
             Some(data) => data as *mut U,
             None => return Err(this),
         };
-        let s = this.s;
-        let permits_acquired = this.permits_acquired;
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        let resource_span = this.resource_span.clone();
-        // NB: Forget to avoid drop impl from being called.
-        mem::forget(this);
+        let this = this.skip_drop();
 
         Ok(RwLockMappedWriteGuard {
-            permits_acquired,
-            s,
+            permits_acquired: this.permits_acquired,
+            s: this.s,
             data,
-            marker: marker::PhantomData,
+            marker: PhantomData,
             #[cfg(all(tokio_unstable, feature = "tracing"))]
-            resource_span,
+            resource_span: this.resource_span,
         })
     }
 }