Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move some things to std::sync::poison and reexport them in std::sync #134692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions library/std/src/sync/barrier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
mod tests;

use crate::fmt;
// FIXME(nonpoison_mutex,nonpoison_condvar): switch to nonpoison versions once they are available
use crate::sync::{Condvar, Mutex};

/// A barrier enables multiple threads to synchronize the beginning
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/sync/lazy_lock.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::once::ExclusiveState;
use super::poison::once::ExclusiveState;
use crate::cell::UnsafeCell;
use crate::mem::ManuallyDrop;
use crate::ops::Deref;
Expand Down Expand Up @@ -63,6 +63,7 @@ union Data<T, F> {
/// ```
#[stable(feature = "lazy_cell", since = "1.80.0")]
pub struct LazyLock<T, F = fn() -> T> {
// FIXME(nonpoison_once): if possible, switch to nonpoison version once it is available
once: Once,
data: UnsafeCell<Data<T, F>>,
}
Expand Down
63 changes: 44 additions & 19 deletions library/std/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@

#![stable(feature = "rust1", since = "1.0.0")]

// No formatting: this file is just re-exports, and their order is worth preserving.
#![cfg_attr(rustfmt, rustfmt::skip)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this skip just get applied to specific blocks, like in https://github.com/rust-lang/rust/blob/addbd001ec56741829f20a3000892f8620dd0843/library/core/src/unicode/mod.rs? Applying to the whole file would probably make it a bit too easy for things to get messy.

Copy link
Contributor Author

@GrigorenkoPV GrigorenkoPV Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then pretty much every block of uses would have to be annotated, because otherwise rustfmt will regroup them.

IMO this looks even noisier and less accurate with all the additional attributes and inability to leave two blank lines between the groups (rustfmt removes double empty lines).



// These come from `core` & `alloc` and only in one flavor: no poisoning.
#[unstable(feature = "exclusive_wrapper", issue = "98407")]
pub use core::sync::Exclusive;
#[stable(feature = "rust1", since = "1.0.0")]
Expand All @@ -175,40 +180,60 @@ pub use core::sync::atomic;
#[stable(feature = "rust1", since = "1.0.0")]
pub use alloc_crate::sync::{Arc, Weak};

// FIXME(sync_nonpoison,sync_poison_mod): remove all `#[doc(inline)]` once the modules are stabilized.

// These exist only in one flavor: no poisoning.
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::barrier::{Barrier, BarrierWaitResult};
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::condvar::{Condvar, WaitTimeoutResult};
#[stable(feature = "lazy_cell", since = "1.80.0")]
pub use self::lazy_lock::LazyLock;
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub use self::mutex::MappedMutexGuard;
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::mutex::{Mutex, MutexGuard};
#[stable(feature = "rust1", since = "1.0.0")]
#[allow(deprecated)]
pub use self::once::{ONCE_INIT, Once, OnceState};
#[stable(feature = "once_cell", since = "1.70.0")]
pub use self::once_lock::OnceLock;
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::poison::{LockResult, PoisonError, TryLockError, TryLockResult};
#[unstable(feature = "reentrant_lock", issue = "121440")]
pub use self::reentrant_lock::{ReentrantLock, ReentrantLockGuard};
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub use self::rwlock::{MappedRwLockReadGuard, MappedRwLockWriteGuard};


// These make sense and exist only with poisoning.
#[stable(feature = "rust1", since = "1.0.0")]
#[doc(inline)]
pub use self::poison::{LockResult, PoisonError};


// These (should) exist in both flavors: with and without poisoning.
// FIXME(sync_nonpoison): implement nonpoison versions:
// * Mutex (nonpoison_mutex)
// * Condvar (nonpoison_condvar)
// * Once (nonpoison_once)
// * RwLock (nonpoison_rwlock)

// FIXME(sync_nonpoison): conditionally select the default flavor based on edition(?).
use self::poison as default_poison_flavor;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are at least two years off from an edition where we would change these defaults, so I don't think this needs to be addressed at this point. Just reexporting from self::poison should be fine, things might change between now and the next edition.


#[stable(feature = "rust1", since = "1.0.0")]
#[doc(inline)]
pub use default_poison_flavor::{
Mutex, MutexGuard, TryLockError, TryLockResult,
Condvar, WaitTimeoutResult,
Once, OnceState,
RwLock, RwLockReadGuard, RwLockWriteGuard,
};
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::rwlock::{RwLock, RwLockReadGuard, RwLockWriteGuard};
#[doc(inline)]
#[expect(deprecated)]
pub use default_poison_flavor::ONCE_INIT;
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
#[doc(inline)]
pub use default_poison_flavor::{MappedMutexGuard, MappedRwLockReadGuard, MappedRwLockWriteGuard};


#[unstable(feature = "mpmc_channel", issue = "126840")]
pub mod mpmc;
pub mod mpsc;

#[unstable(feature = "sync_poison_mod", issue = "134646")]
pub mod poison;

mod barrier;
mod condvar;
mod lazy_lock;
mod mutex;
pub(crate) mod once;
mod once_lock;
mod poison;
mod reentrant_lock;
mod rwlock;
1 change: 1 addition & 0 deletions library/std/src/sync/once_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ use crate::sync::Once;
/// ```
#[stable(feature = "once_cell", since = "1.70.0")]
pub struct OnceLock<T> {
// FIXME(nonpoison_once): switch to nonpoison version once it is available
once: Once,
// Whether or not the value is initialized is tracked by `once.is_completed()`.
value: UnsafeCell<MaybeUninit<T>>,
Expand Down
87 changes: 84 additions & 3 deletions library/std/src/sync/poison.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,92 @@
//! Synchronization objects that employ poisoning.
//!
//! # Poisoning
//!
//! All synchronization objects in this module implement a strategy called "poisoning"
//! where if a thread panics while holding the exclusive access granted by the primitive,
//! the state of the primitive is set to "poisoned".
//! This information is then propagated to all other threads
//! to signify that the data protected by this primitive is likely tainted
//! (some invariant is not being upheld).
//!
//! The specifics of how this "poisoned" state affects other threads
//! depend on the primitive. See [#Overview] bellow.
//!
//! For the alternative implementations that do not employ poisoning,
//! see `std::sys::nonpoisoning`.
//!
//! # Overview
//!
//! Below is a list of synchronization objects provided by this module
//! with a high-level overview for each object and a description
//! of how it employs "poisoning".
//!
//! - [`Condvar`]: Condition Variable, providing the ability to block
//! a thread while waiting for an event to occur.
//!
//! Condition variables are typically associated with
//! a boolean predicate (a condition) and a mutex.
//! This implementation is associated with [`poison::Mutex`](Mutex),
//! which employs poisoning.
//! For this reason, [`Condvar::wait()`] will return a [`LockResult`],
//! just like [`poison::Mutex::lock()`](Mutex::lock) does.
//!
//! - [`Mutex`]: Mutual Exclusion mechanism, which ensures that at
//! most one thread at a time is able to access some data.
//!
//! [`Mutex::lock()`] returns a [`LockResult`],
//! providing a way to deal with the poisoned state.
//! See [`Mutex`'s documentation](Mutex#poisoning) for more.
//!
//! - [`Once`]: A thread-safe way to run a piece of code only once.
//! Mostly useful for implementing one-time global initialization.
//!
//! [`Once`] is poisoned if the piece of code passed to
//! [`Once::call_once()`] or [`Once::call_once_force()`] panics.
//! When in poisoned state, subsequent calls to [`Once::call_once()`] will panic too.
//! [`Once::call_once_force()`] can be used to clear the poisoned state.
//!
//! - [`RwLock`]: Provides a mutual exclusion mechanism which allows
//! multiple readers at the same time, while allowing only one
//! writer at a time. In some cases, this can be more efficient than
//! a mutex.
//!
//! This implementation, like [`Mutex`], will become poisoned on a panic.
//! Note, however, that an `RwLock` may only be poisoned if a panic occurs
//! while it is locked exclusively (write mode). If a panic occurs in any reader,
//! then the lock will not be poisoned.
// FIXME(sync_nonpoison) add links to sync::nonpoison to the doc comment above.

#[stable(feature = "rust1", since = "1.0.0")]
pub use self::condvar::{Condvar, WaitTimeoutResult};
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub use self::mutex::MappedMutexGuard;
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::mutex::{Mutex, MutexGuard};
#[stable(feature = "rust1", since = "1.0.0")]
#[expect(deprecated)]
pub use self::once::ONCE_INIT;
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::once::{Once, OnceState};
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub use self::rwlock::{MappedRwLockReadGuard, MappedRwLockWriteGuard};
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::rwlock::{RwLock, RwLockReadGuard, RwLockWriteGuard};
use crate::error::Error;
use crate::fmt;
#[cfg(panic = "unwind")]
use crate::sync::atomic::{AtomicBool, Ordering};
#[cfg(panic = "unwind")]
use crate::thread;

pub struct Flag {
mod condvar;
#[stable(feature = "rust1", since = "1.0.0")]
mod mutex;
pub(crate) mod once;
mod rwlock;

pub(crate) struct Flag {
#[cfg(panic = "unwind")]
failed: AtomicBool,
}
Expand Down Expand Up @@ -78,7 +159,7 @@ impl Flag {
}

#[derive(Clone)]
pub struct Guard {
pub(crate) struct Guard {
#[cfg(panic = "unwind")]
panicking: bool,
}
Expand Down Expand Up @@ -316,7 +397,7 @@ impl<T> Error for TryLockError<T> {
}
}

pub fn map_result<T, U, F>(result: LockResult<T>, f: F) -> LockResult<U>
pub(crate) fn map_result<T, U, F>(result: LockResult<T>, f: F) -> LockResult<U>
where
F: FnOnce(T) -> U,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
mod tests;

use crate::fmt;
use crate::sync::{LockResult, MutexGuard, PoisonError, mutex, poison};
use crate::sync::poison::{self, LockResult, MutexGuard, PoisonError, mutex};
use crate::sys::sync as sys;
use crate::time::{Duration, Instant};

Expand All @@ -16,6 +16,8 @@ use crate::time::{Duration, Instant};
#[stable(feature = "wait_timeout", since = "1.5.0")]
pub struct WaitTimeoutResult(bool);

// FIXME(sync_nonpoison): `WaitTimeoutResult` is actually poisoning-agnostic, it seems.
// Should we take advantage of this fact?
impl WaitTimeoutResult {
/// Returns `true` if the wait was known to have timed out.
///
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion library/std/src/sys/sync/once/futex.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::cell::Cell;
use crate::sync as public;
use crate::sync::atomic::Ordering::{Acquire, Relaxed, Release};
use crate::sync::once::ExclusiveState;
use crate::sync::poison::once::ExclusiveState;
use crate::sys::futex::{Futex, Primitive, futex_wait, futex_wake_all};

// On some platforms, the OS is very nice and handles the waiter queue for us.
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/sync/once/no_threads.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::cell::Cell;
use crate::sync as public;
use crate::sync::once::ExclusiveState;
use crate::sync::poison::once::ExclusiveState;

pub struct Once {
state: Cell<State>,
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/sync/once/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
use crate::cell::Cell;
use crate::sync::atomic::Ordering::{AcqRel, Acquire, Release};
use crate::sync::atomic::{AtomicBool, AtomicPtr};
use crate::sync::once::ExclusiveState;
use crate::sync::poison::once::ExclusiveState;
use crate::thread::{self, Thread};
use crate::{fmt, ptr, sync as public};

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/traits/const-traits/span-bug-issue-121418.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ LL | pub const fn new() -> std::sync::Mutex<dyn T> {}
|
= help: within `Mutex<(dyn T + 'static)>`, the trait `Sized` is not implemented for `(dyn T + 'static)`
note: required because it appears within the type `Mutex<(dyn T + 'static)>`
--> $SRC_DIR/std/src/sync/mutex.rs:LL:COL
--> $SRC_DIR/std/src/sync/poison/mutex.rs:LL:COL
= note: the return type of a function must have a statically known size

error: aborting due to 3 previous errors
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/typeck/assign-non-lval-derefmut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ LL | x.lock().unwrap() += 1;
| cannot use `+=` on type `MutexGuard<'_, usize>`
|
note: the foreign item type `MutexGuard<'_, usize>` doesn't implement `AddAssign<{integer}>`
--> $SRC_DIR/std/src/sync/mutex.rs:LL:COL
--> $SRC_DIR/std/src/sync/poison/mutex.rs:LL:COL
|
= note: not implement `AddAssign<{integer}>`
help: `+=` can be used on `usize` if you dereference the left-hand side
Expand Down Expand Up @@ -52,7 +52,7 @@ LL | y += 1;
| cannot use `+=` on type `MutexGuard<'_, usize>`
|
note: the foreign item type `MutexGuard<'_, usize>` doesn't implement `AddAssign<{integer}>`
--> $SRC_DIR/std/src/sync/mutex.rs:LL:COL
--> $SRC_DIR/std/src/sync/poison/mutex.rs:LL:COL
|
= note: not implement `AddAssign<{integer}>`
help: `+=` can be used on `usize` if you dereference the left-hand side
Expand Down
Loading