summaryrefslogtreecommitdiffstats
path: root/tokio/src/sync/batch_semaphore.rs
diff options
context:
space:
mode:
authorEliza Weisman <eliza@buoyant.io>2020-04-03 15:45:29 -0700
committerGitHub <noreply@github.com>2020-04-03 15:45:29 -0700
commit1121a8eb23f6f0edd9f41cdc08331d40e18105ee (patch)
treee2420ae1dd6504ab1f4fb313aefa5a96a7dff12e /tokio/src/sync/batch_semaphore.rs
parent6fa40b6e2060b253dd32df326440fa6196763412 (diff)
sync: ensure Mutex, RwLock, and Semaphore futures are Send + Sync (#2375)
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.
Diffstat (limited to 'tokio/src/sync/batch_semaphore.rs')
-rw-r--r--tokio/src/sync/batch_semaphore.rs7
1 files changed, 7 insertions, 0 deletions
diff --git a/tokio/src/sync/batch_semaphore.rs b/tokio/src/sync/batch_semaphore.rs
index 5d15311d..436737a6 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 {