From ed584de97df7a06058a6ff9be9834a4ddcd3655d Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 3 Apr 2020 12:22:21 -0700 Subject: [PATCH 1/5] sync: Add bounds checks for Send + Sync futures Signed-off-by: Eliza Weisman --- tokio/src/sync/mutex.rs | 7 +++++++ tokio/src/sync/rwlock.rs | 7 +++++++ tokio/src/sync/semaphore.rs | 7 +++++++ 3 files changed, 21 insertions(+) diff --git a/tokio/src/sync/mutex.rs b/tokio/src/sync/mutex.rs index dac5ac16c42..38ff1d7d5c8 100644 --- a/tokio/src/sync/mutex.rs +++ b/tokio/src/sync/mutex.rs @@ -137,8 +137,15 @@ impl Error for TryLockError {} fn bounds() { fn check_send() {} fn check_unpin() {} + // This has to take a value, since the async fn's return type is unnameable. + fn check_send_sync_val(t: T) {} + fn check_send_sync() {} check_send::>(); check_unpin::>(); + check_send_sync::>(); + + let mutex = Mutex::new(1); + check_send_sync_val(mutex.lock()); } impl Mutex { diff --git a/tokio/src/sync/rwlock.rs b/tokio/src/sync/rwlock.rs index 7cce69a5c5d..0a4ec472e52 100644 --- a/tokio/src/sync/rwlock.rs +++ b/tokio/src/sync/rwlock.rs @@ -133,6 +133,9 @@ fn bounds() { fn check_send() {} fn check_sync() {} fn check_unpin() {} + // This has to take a value, since the async fn's return type is unnameable. + fn check_send_sync_val(t: T) {} + check_send::>(); check_sync::>(); check_unpin::>(); @@ -142,6 +145,10 @@ fn bounds() { check_sync::>(); check_unpin::>(); + + let rwlock = RwLock::new(0); + check_send_sync_val(rwlock.read()); + check_send_sync_val(rwlock.write()); } // As long as T: Send + Sync, it's fine to send and share RwLock between threads. diff --git a/tokio/src/sync/semaphore.rs b/tokio/src/sync/semaphore.rs index e34e49cc7fe..e58db987e99 100644 --- a/tokio/src/sync/semaphore.rs +++ b/tokio/src/sync/semaphore.rs @@ -39,8 +39,15 @@ pub struct TryAcquireError(()); #[cfg(not(loom))] fn bounds() { fn check_unpin() {} + // This has to take a value, since the async fn's return type is unnameable. + fn check_send_sync_val(t: T) {} + fn check_send_sync() {} check_unpin::(); check_unpin::>(); + check_send_sync::(); + + let semaphore = Semaphore::new(0); + check_send_sync_val(semaphore.acquire()); } impl Semaphore { From 703fc649e8c2ad3d28a5fcb033162e0b823f714e Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 3 Apr 2020 12:23:52 -0700 Subject: [PATCH 2/5] sync: ensure Mutex, RwLock, and Semaphore futures are Send + Sync Previously, the `Mutex::lock`, `RwLock::{read, write}`, and `Semaphore::acquire` futures in `tokio::sync` implemented `Send + Sync` automatically. This was by virtue of being implemented using a `poll_fn` that only closed over `Send + Sync` types. However, this broke in PR #2325, which rewrote those types using the new `batch_semaphore`. Now, they await an `Acquire` future, which contains a `Waiter`, which internally contains an `UnsafeCell`, and thus does not implement `Sync`. Since removing previously implemented traits breaks existing code, this inadvertantly caused a breaking change. There were tests ensuring that the `Mutex`, `RwLock`, and `Semaphore` types themselves were `Send + Sync`, but no tests that the _futures they return_ implemented those traits. I've fixed this by adding an explicit impl of `Sync` for the `batch_semaphore::Acquire` future. Since the `Waiter` type held by this struct is only accessed when borrowed mutably, it is safe for it to implement `Sync`. Additionally, I've added to the bounds checks for the effected `tokio::sync` types to ensure that returned futures continue to implement `Send + Sync` in the future. Signed-off-by: Eliza Weisman --- tokio/src/sync/batch_semaphore.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tokio/src/sync/batch_semaphore.rs b/tokio/src/sync/batch_semaphore.rs index 5d15311da0c..436737a6709 100644 --- a/tokio/src/sync/batch_semaphore.rs +++ b/tokio/src/sync/batch_semaphore.rs @@ -463,6 +463,13 @@ impl Drop for Acquire<'_> { } } +// Safety: the `Acquire` future is not `Sync` automatically because it contains +// a `Waiter`, which, in turn, contains an `UnsafeCell`. However, the +// `UnsafeCell` is only accessed when the future is borrowed mutably (either in +// `poll` or in `drop`). Therefore, it is safe (although not particularly +// _useful_) for the future to be borrowed immutably across threads. +unsafe impl Sync for Acquire<'_> {} + // ===== impl AcquireError ==== impl AcquireError { From ed6ea4ed392e262afd7eda870d81eb842cf0c874 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 3 Apr 2020 12:54:40 -0700 Subject: [PATCH 3/5] fix unused variable in checks Signed-off-by: Eliza Weisman --- tokio/src/sync/mutex.rs | 2 +- tokio/src/sync/rwlock.rs | 2 +- tokio/src/sync/semaphore.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tokio/src/sync/mutex.rs b/tokio/src/sync/mutex.rs index 38ff1d7d5c8..6b51b405681 100644 --- a/tokio/src/sync/mutex.rs +++ b/tokio/src/sync/mutex.rs @@ -138,7 +138,7 @@ fn bounds() { fn check_send() {} fn check_unpin() {} // This has to take a value, since the async fn's return type is unnameable. - fn check_send_sync_val(t: T) {} + fn check_send_sync_val(_t: T) {} fn check_send_sync() {} check_send::>(); check_unpin::>(); diff --git a/tokio/src/sync/rwlock.rs b/tokio/src/sync/rwlock.rs index 0a4ec472e52..0f7991a5bf8 100644 --- a/tokio/src/sync/rwlock.rs +++ b/tokio/src/sync/rwlock.rs @@ -134,7 +134,7 @@ fn bounds() { fn check_sync() {} fn check_unpin() {} // This has to take a value, since the async fn's return type is unnameable. - fn check_send_sync_val(t: T) {} + fn check_send_sync_val(_t: T) {} check_send::>(); check_sync::>(); diff --git a/tokio/src/sync/semaphore.rs b/tokio/src/sync/semaphore.rs index e58db987e99..4cce7e8f5bc 100644 --- a/tokio/src/sync/semaphore.rs +++ b/tokio/src/sync/semaphore.rs @@ -40,7 +40,7 @@ pub struct TryAcquireError(()); fn bounds() { fn check_unpin() {} // This has to take a value, since the async fn's return type is unnameable. - fn check_send_sync_val(t: T) {} + fn check_send_sync_val(_t: T) {} fn check_send_sync() {} check_unpin::(); check_unpin::>(); From 89fcffb62682a5bb703f88163fc7c8aa5c07d75f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 3 Apr 2020 13:26:41 -0700 Subject: [PATCH 4/5] prepare to release 0.2.16 Signed-off-by: Eliza Weisman --- tokio/CHANGELOG.md | 8 ++++++++ tokio/Cargo.toml | 4 ++-- tokio/src/lib.rs | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tokio/CHANGELOG.md b/tokio/CHANGELOG.md index b748408e088..68e2af6583f 100644 --- a/tokio/CHANGELOG.md +++ b/tokio/CHANGELOG.md @@ -1,3 +1,11 @@ +# 0.2.16 (April 3, 2020) + +### Fixes + +- sync: fix a regression where `Mutex`, `Semaphore`, and `RwLock` futures no + longer implement `Sync` (#2375) + + # 0.2.15 (April 2, 2020) ### Fixes diff --git a/tokio/Cargo.toml b/tokio/Cargo.toml index 31bf3271213..5dc264b5368 100644 --- a/tokio/Cargo.toml +++ b/tokio/Cargo.toml @@ -8,12 +8,12 @@ name = "tokio" # - README.md # - Update CHANGELOG.md. # - Create "v0.2.x" git tag. -version = "0.2.15" +version = "0.2.16" edition = "2018" authors = ["Tokio Contributors "] license = "MIT" readme = "README.md" -documentation = "https://docs.rs/tokio/0.2.15/tokio/" +documentation = "https://docs.rs/tokio/0.1.16/tokio/" repository = "https://github.com/tokio-rs/tokio" homepage = "https://tokio.rs" description = """ diff --git a/tokio/src/lib.rs b/tokio/src/lib.rs index 7db4cb3045f..dad00fd76c4 100644 --- a/tokio/src/lib.rs +++ b/tokio/src/lib.rs @@ -1,4 +1,4 @@ -#![doc(html_root_url = "https://docs.rs/tokio/0.2.15")] +#![doc(html_root_url = "https://docs.rs/tokio/0.1.16")] #![allow( clippy::cognitive_complexity, clippy::large_enum_variant, From ed8882e31c99e151a796a262d314a9077f889cad Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 3 Apr 2020 13:44:19 -0700 Subject: [PATCH 5/5] fix wrong version whoops, thanks carl! Co-Authored-By: Carl Lerche --- tokio/Cargo.toml | 2 +- tokio/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tokio/Cargo.toml b/tokio/Cargo.toml index 5dc264b5368..9fd5f95de6f 100644 --- a/tokio/Cargo.toml +++ b/tokio/Cargo.toml @@ -13,7 +13,7 @@ edition = "2018" authors = ["Tokio Contributors "] license = "MIT" readme = "README.md" -documentation = "https://docs.rs/tokio/0.1.16/tokio/" +documentation = "https://docs.rs/tokio/0.2.16/tokio/" repository = "https://github.com/tokio-rs/tokio" homepage = "https://tokio.rs" description = """ diff --git a/tokio/src/lib.rs b/tokio/src/lib.rs index dad00fd76c4..d0cb5fa4230 100644 --- a/tokio/src/lib.rs +++ b/tokio/src/lib.rs @@ -1,4 +1,4 @@ -#![doc(html_root_url = "https://docs.rs/tokio/0.1.16")] +#![doc(html_root_url = "https://docs.rs/tokio/0.2.16")] #![allow( clippy::cognitive_complexity, clippy::large_enum_variant,