Skip to content

Request: Make std::marker::Freeze pub again #60715

Open

Description

I had heard tell of Freeze but didn't really know what it was until today. swym, a hybrid transactional memory library, has an accidental reimplementation of Freeze using optin_builtin_traits. Unfortunately optin_builtin_traits is the only feature keeping swym on nightly.

The ticket that removed Freeze doesn't have much of an explanation for why it was removed so I'm assuming it was a lack of motivating use cases.

Use Case

swym::tcell::TCell::borrow returns snapshots of data - shallow memcpys - that are guaranteed to not be torn, and be valid for the duration of the transaction. Those snapshots hold on to the lifetime of the TCell in order to act like a true reference, without blocking updates to the TCell from other threads. Other threads promise to not mutate the value that had its snapshot taken until the transaction has finished, but are permitted to move the value in memory.

This works great for a lot of types, but fails miserably when UnsafeCells are directly stored in the type.

let x = TCell::new(Mutex::new("hello there".to_owned()));

// ..  inside a transaction
let shallow_copy = x.borrow(tx, Default::default())?;
// locking a shallow copy of a lock... is not really a lock at all!
*shallow_copy.lock().unwrap() = "uh oh".to_owned();

Even if Mutex internally had a pointer to the "actual" mutex data structure, the above example would still be broken because the String is deallocated through the shallow copy. The String contained in the TCell would point to freed memory.

Note that having TCell::borrow require Sync would still allow the above broken example to compile.

Freeze

If swym::tcell::TCell::borrow could require Freeze then this would not be an issue as the Mutex type is definitely not Freeze.

pub(crate) unsafe auto trait Freeze {}

impl<T: ?Sized> !Freeze for UnsafeCell<T> {}
unsafe impl<T: ?Sized> Freeze for PhantomData<T> {}
unsafe impl<T: ?Sized> Freeze for *const T {}
unsafe impl<T: ?Sized> Freeze for *mut T {}
unsafe impl<T: ?Sized> Freeze for &T {}
unsafe impl<T: ?Sized> Freeze for &mut T {}

Shallow immutability is all that is required for TCell::borrow to work. Sync is only necessary to make TCell Sync.

  • TCell<String> - should be permitted.
  • TCell<Mutex<String>> - should be forbidden.
  • TCell<Box<Mutex<String>>> - should be permitted.

Alternatives

  • A manually implemented marker trait could work, but is actually very dangerous in practice. In the below example, assume that the impl of MyFreeze was correct when it was written. Everytime the author of MyType updates their dependency on other_crate they must recheck that OtherType still has no direct interior mutability or risk unsoundness.
struct MyType { value: other_crate::OtherType }
unsafe impl MyFreeze for MyType {}
  • Add a T: Copy bound on TCell::<T>::borrow. This would definitely work but leaves a lot of types out.

  • Wait for OIBITs to stabilize (assuming it will be stabilized).

  • Have TCell store a Box<T> internally, and only work with heap allocated data where interior mutability is of no concern. This would be pretty effective, and if the type is small enough and Copy, the Box could be elided. While not as good as stabilizing Freeze, I think this is the best alternative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    C-feature-requestCategory: A feature request, i.e: not implemented / a PR.Category: A feature request, i.e: not implemented / a PR.T-langRelevant to the language team, which will review and decide on the PR/issue.Relevant to the language team, which will review and decide on the PR/issue.needs-rfcThis change is large or controversial enough that it should have an RFC accepted before doing it.This change is large or controversial enough that it should have an RFC accepted before doing it.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions