From 993dc5e5ade853794bcfb6408b6e9614d46e2044 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 30 Dec 2021 18:19:41 +0100 Subject: [PATCH 1/6] Add CI test --- .github/workflows/ci.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4474e9667bd..8110390cdb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,7 @@ jobs: needs: - test - test-unstable + - test-parking_lot - miri - cross - features @@ -77,6 +78,20 @@ jobs: # bench.yml workflow runs benchmarks only on linux. if: startsWith(matrix.os, 'ubuntu') + test-parking_lot: + name: compile tests with parking lot send guards + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Install Rust + run: rustup update stable + - uses: Swatinem/rust-cache@v1 + - name: Enable parking_lot send_guard feature + # Inserts the line "plsend = ["parking_lot/send_guard"]" right after [features] + run: sed -i '/\[features\]/a plsend = ["parking_lot/send_guard"]' tokio/Cargo.toml + - name: Compile tests with all features enabled + run: cargo build --workspace --all-features --tests + valgrind: name: valgrind runs-on: ubuntu-latest From 0f31dc0021981d5122e046a38f69b7019b946da2 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 30 Dec 2021 18:37:24 +0100 Subject: [PATCH 2/6] Add PhantomData to parking_lot mocks --- tokio/src/loom/std/parking_lot.rs | 80 +++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 19 deletions(-) diff --git a/tokio/src/loom/std/parking_lot.rs b/tokio/src/loom/std/parking_lot.rs index 8448bed53d7..1f20ccd365c 100644 --- a/tokio/src/loom/std/parking_lot.rs +++ b/tokio/src/loom/std/parking_lot.rs @@ -3,83 +3,125 @@ //! //! This can be extended to additional types/methods as required. +use std::marker::PhantomData; +use std::ops::{Deref, DerefMut}; use std::sync::LockResult; use std::time::Duration; // Types that do not need wrapping -pub(crate) use parking_lot::{MutexGuard, RwLockReadGuard, RwLockWriteGuard, WaitTimeoutResult}; +pub(crate) use parking_lot::WaitTimeoutResult; -/// Adapter for `parking_lot::Mutex` to the `std::sync::Mutex` interface. #[derive(Debug)] -pub(crate) struct Mutex(parking_lot::Mutex); +pub(crate) struct Mutex(PhantomData>, parking_lot::Mutex); #[derive(Debug)] -pub(crate) struct RwLock(parking_lot::RwLock); +pub(crate) struct RwLock(PhantomData>, parking_lot::RwLock); -/// Adapter for `parking_lot::Condvar` to the `std::sync::Condvar` interface. #[derive(Debug)] -pub(crate) struct Condvar(parking_lot::Condvar); +pub(crate) struct Condvar(PhantomData, parking_lot::Condvar); + +#[derive(Debug)] +pub(crate) struct MutexGuard<'a, T: ?Sized>(PhantomData>, parking_lot::MutexGuard<'a, T>); + +#[derive(Debug)] +pub(crate) struct RwLockReadGuard<'a, T: ?Sized>(PhantomData>, parking_lot::RwLockReadGuard<'a, T>); + +#[derive(Debug)] +pub(crate) struct RwLockWriteGuard<'a, T: ?Sized>(PhantomData>, parking_lot::RwLockWriteGuard<'a, T>); impl Mutex { #[inline] pub(crate) fn new(t: T) -> Mutex { - Mutex(parking_lot::Mutex::new(t)) + Mutex(PhantomData, parking_lot::Mutex::new(t)) } #[inline] #[cfg(all(feature = "parking_lot", not(all(loom, test)),))] #[cfg_attr(docsrs, doc(cfg(all(feature = "parking_lot",))))] pub(crate) const fn const_new(t: T) -> Mutex { - Mutex(parking_lot::const_mutex(t)) + Mutex(PhantomData, parking_lot::const_mutex(t)) } #[inline] pub(crate) fn lock(&self) -> MutexGuard<'_, T> { - self.0.lock() + MutexGuard(PhantomData, self.1.lock()) } #[inline] pub(crate) fn try_lock(&self) -> Option> { - self.0.try_lock() + self.1.try_lock().map(|guard| MutexGuard(PhantomData, guard)) } #[inline] pub(crate) fn get_mut(&mut self) -> &mut T { - self.0.get_mut() + self.1.get_mut() } // Note: Additional methods `is_poisoned` and `into_inner`, can be // provided here as needed. } +impl<'a, T: ?Sized> Deref for MutexGuard<'a, T> { + type Target = T; + fn deref(&self) -> &T { + self.1.deref() + } +} + +impl<'a, T: ?Sized> DerefMut for MutexGuard<'a, T> { + fn deref_mut(&mut self) -> &mut T { + self.1.deref_mut() + } +} + impl RwLock { pub(crate) fn new(t: T) -> RwLock { - RwLock(parking_lot::RwLock::new(t)) + RwLock(PhantomData, parking_lot::RwLock::new(t)) } pub(crate) fn read(&self) -> LockResult> { - Ok(self.0.read()) + Ok(RwLockReadGuard(PhantomData, self.1.read())) } pub(crate) fn write(&self) -> LockResult> { - Ok(self.0.write()) + Ok(RwLockWriteGuard(PhantomData, self.1.write())) + } +} + +impl<'a, T: ?Sized> Deref for RwLockReadGuard<'a, T> { + type Target = T; + fn deref(&self) -> &T { + self.1.deref() + } +} + +impl<'a, T: ?Sized> Deref for RwLockWriteGuard<'a, T> { + type Target = T; + fn deref(&self) -> &T { + self.1.deref() + } +} + +impl<'a, T: ?Sized> DerefMut for RwLockWriteGuard<'a, T> { + fn deref_mut(&mut self) -> &mut T { + self.1.deref_mut() } } impl Condvar { #[inline] pub(crate) fn new() -> Condvar { - Condvar(parking_lot::Condvar::new()) + Condvar(PhantomData, parking_lot::Condvar::new()) } #[inline] pub(crate) fn notify_one(&self) { - self.0.notify_one(); + self.1.notify_one(); } #[inline] pub(crate) fn notify_all(&self) { - self.0.notify_all(); + self.1.notify_all(); } #[inline] @@ -87,7 +129,7 @@ impl Condvar { &self, mut guard: MutexGuard<'a, T>, ) -> LockResult> { - self.0.wait(&mut guard); + self.1.wait(&mut guard.1); Ok(guard) } @@ -97,7 +139,7 @@ impl Condvar { mut guard: MutexGuard<'a, T>, timeout: Duration, ) -> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> { - let wtr = self.0.wait_for(&mut guard, timeout); + let wtr = self.1.wait_for(&mut guard.1, timeout); Ok((guard, wtr)) } From d15c9e491ca986fb2f7eb0d09e528dca33ef9859 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 30 Dec 2021 18:38:29 +0100 Subject: [PATCH 3/6] rustfmt --- tokio/src/loom/std/parking_lot.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tokio/src/loom/std/parking_lot.rs b/tokio/src/loom/std/parking_lot.rs index 1f20ccd365c..3a785914dd2 100644 --- a/tokio/src/loom/std/parking_lot.rs +++ b/tokio/src/loom/std/parking_lot.rs @@ -21,13 +21,22 @@ pub(crate) struct RwLock(PhantomData>, parking_lot::RwLo pub(crate) struct Condvar(PhantomData, parking_lot::Condvar); #[derive(Debug)] -pub(crate) struct MutexGuard<'a, T: ?Sized>(PhantomData>, parking_lot::MutexGuard<'a, T>); +pub(crate) struct MutexGuard<'a, T: ?Sized>( + PhantomData>, + parking_lot::MutexGuard<'a, T>, +); #[derive(Debug)] -pub(crate) struct RwLockReadGuard<'a, T: ?Sized>(PhantomData>, parking_lot::RwLockReadGuard<'a, T>); +pub(crate) struct RwLockReadGuard<'a, T: ?Sized>( + PhantomData>, + parking_lot::RwLockReadGuard<'a, T>, +); #[derive(Debug)] -pub(crate) struct RwLockWriteGuard<'a, T: ?Sized>(PhantomData>, parking_lot::RwLockWriteGuard<'a, T>); +pub(crate) struct RwLockWriteGuard<'a, T: ?Sized>( + PhantomData>, + parking_lot::RwLockWriteGuard<'a, T>, +); impl Mutex { #[inline] @@ -49,7 +58,9 @@ impl Mutex { #[inline] pub(crate) fn try_lock(&self) -> Option> { - self.1.try_lock().map(|guard| MutexGuard(PhantomData, guard)) + self.1 + .try_lock() + .map(|guard| MutexGuard(PhantomData, guard)) } #[inline] From 5c7dea206f45cb1c673b573af0314b6689eae739 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 30 Dec 2021 18:41:53 +0100 Subject: [PATCH 4/6] Add comment --- tokio/src/loom/std/parking_lot.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tokio/src/loom/std/parking_lot.rs b/tokio/src/loom/std/parking_lot.rs index 3a785914dd2..88eb525f64d 100644 --- a/tokio/src/loom/std/parking_lot.rs +++ b/tokio/src/loom/std/parking_lot.rs @@ -8,6 +8,12 @@ use std::ops::{Deref, DerefMut}; use std::sync::LockResult; use std::time::Duration; +// All types in this file are marked with PhantomData to ensure that +// parking_lot's send_guard feature does not leak through and affect when Tokio +// types are Send. +// +// See for more info. + // Types that do not need wrapping pub(crate) use parking_lot::WaitTimeoutResult; From 9461a44dbc21a962448fef9e35a39379e70201bd Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 30 Dec 2021 18:48:28 +0100 Subject: [PATCH 5/6] Add another comment --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8110390cdb1..69ae57282e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,13 @@ jobs: if: startsWith(matrix.os, 'ubuntu') test-parking_lot: + # The parking_lot crate has a feature called send_guard which changes when + # some of its types are Send. Tokio has some measures in place to prevent + # this from affecting when Tokio types are Send, and this test exists to + # ensure that those measures are working. + # + # This relies on the potentially affected Tokio type being listed in + # `tokio/tokio/tests/async_send_sync.rs`. name: compile tests with parking lot send guards runs-on: ubuntu-latest steps: From 0f240a9f5a560cbec2a01d85c78d3258f75dcdad Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 30 Dec 2021 20:01:26 +0100 Subject: [PATCH 6/6] Add Display impls --- tokio/src/loom/std/parking_lot.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tokio/src/loom/std/parking_lot.rs b/tokio/src/loom/std/parking_lot.rs index 88eb525f64d..034a0ce69a5 100644 --- a/tokio/src/loom/std/parking_lot.rs +++ b/tokio/src/loom/std/parking_lot.rs @@ -3,6 +3,7 @@ //! //! This can be extended to additional types/methods as required. +use std::fmt; use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; use std::sync::LockResult; @@ -163,3 +164,21 @@ impl Condvar { // Note: Additional methods `wait_timeout_ms`, `wait_timeout_until`, // `wait_until` can be provided here as needed. } + +impl<'a, T: ?Sized + fmt::Display> fmt::Display for MutexGuard<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.1, f) + } +} + +impl<'a, T: ?Sized + fmt::Display> fmt::Display for RwLockReadGuard<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.1, f) + } +} + +impl<'a, T: ?Sized + fmt::Display> fmt::Display for RwLockWriteGuard<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.1, f) + } +}