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

Replace UnsafeCell with MaybeUnit #53

Closed
wants to merge 3 commits into from
Closed
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
6 changes: 3 additions & 3 deletions .github/bors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ block_labels = ["needs-decision"]
delete_merged_branches = true
required_approvals = 1
status = [
"clippy (1.54)",
"clippy (1.55)",
"clippy (1.63)",
"clippy (1.63, std)",
"rustfmt",
"test (1.54)",
"test (1.55)",
"test (1.63)",
"test (1.63, std)",
]
]
2 changes: 1 addition & 1 deletion .github/workflows/clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
strategy:
matrix:
include:
- rust: 1.54
- rust: 1.55
features: ''
- rust: 1.63
features: ''
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
strategy:
matrix:
include:
- rust: 1.54
- rust: 1.55
features: ''
- rust: 1.63
features: ''
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

No unreleased changes yet
- Use `MaybeUninit` instead of `UnsafeCell`, internally.
- MSRV changed to `1.55` when `std` feature is disabled.

## [v1.1.3] - 2024-08-22

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ which would be unsound.

This crate is guaranteed to compile on the following Rust versions:

- If the `std` feature is not enabled: stable Rust 1.54 and up.
- If the `std` feature is not enabled: stable Rust 1.55 and up.
- If the `std` feature is enabled: stable Rust 1.63 and up.

It might compile with older versions but that may change in any new patch release.
Expand Down
37 changes: 31 additions & 6 deletions src/mutex.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use super::CriticalSection;
use core::cell::{Ref, RefCell, RefMut, UnsafeCell};
use core::{
cell::{Ref, RefCell, RefMut},
mem::MaybeUninit,
};

/// A mutex based on critical sections.
///
Expand Down Expand Up @@ -72,15 +75,23 @@ use core::cell::{Ref, RefCell, RefMut, UnsafeCell};
/// [interior mutability]: https://doc.rust-lang.org/reference/interior-mutability.html
#[derive(Debug)]
pub struct Mutex<T> {
inner: UnsafeCell<T>,
// The `MaybeUninit` is not strictly necessary here: In theory, just using `T` should
// be fine.
// However, without `MaybeUninit`, the compiler may use niches inside `T`, and may
// read the niche value _without locking the mutex_. As we don't provide interior
// mutability, this is still not violating any aliasing rules and should be perfectly
// fine. But as the cost of adding `MaybeUninit` is very small, we add it out of
// cautiousness, just in case the reason `T` is not `Sync` in the first place is
// something very obscure we didn't consider.
inner: MaybeUninit<T>,
}

impl<T> Mutex<T> {
/// Creates a new mutex.
#[inline]
pub const fn new(value: T) -> Self {
Mutex {
inner: UnsafeCell::new(value),
inner: MaybeUninit::new(value),
}
}

Expand All @@ -92,19 +103,24 @@ impl<T> Mutex<T> {
/// unwanted optimizations.
#[inline]
pub fn get_mut(&mut self) -> &mut T {
unsafe { &mut *self.inner.get() }
// Safety: inner is always initialized
unsafe { self.inner.assume_init_mut() }
}

/// Unwraps the contained value, consuming the mutex.
#[inline]
pub fn into_inner(self) -> T {
self.inner.into_inner()
// Safety:
// - inner is always initialized
// - self will be dropped at the end of the function, _not_ dropping the contents of MaybeUninit
unsafe { self.inner.as_ptr().read() }
}

/// Borrows the data for the duration of the critical section.
#[inline]
pub fn borrow<'cs>(&'cs self, _cs: CriticalSection<'cs>) -> &'cs T {
unsafe { &*self.inner.get() }
// Safety: inner is always initialized
unsafe { self.inner.assume_init_ref() }
}
}

Expand Down Expand Up @@ -185,6 +201,15 @@ impl<T: Default> Mutex<RefCell<T>> {
}
}

impl<T> Drop for Mutex<T> {
fn drop(&mut self) {
// Safety:
// - inner is always initialized
// - self will be dropped at the end of the function, _not_ dropping the contents of MaybeUninit
core::mem::drop(unsafe { self.inner.as_ptr().read() });
}
}

// NOTE A `Mutex` can be used as a channel so the protected data must be `Send`
// to prevent sending non-Sendable stuff (e.g. access tokens) across different
// threads.
Expand Down
Loading