Tracking Issue for SyncUnsafeCell

[m-ou-se]

View all comments

Feature gate: #![feature(sync_unsafe_cell)]

This is a tracking issue for SyncUnsafeCell.

Public API

// core::cell

#[repr(transparent)]
pub struct SyncUnsafeCell<T: ?Sized> {
    value: UnsafeCell<T>,
}

// Bound `T: Sync`, unresolved if we want this
unsafe impl<T: ?Sized + Sync> Sync for SyncUnsafeCell<T> {}

// Identical interface as UnsafeCell:

impl<T> SyncUnsafeCell<T> {
    pub const fn new(value: T) -> Self;
    pub const fn into_inner(self) -> T;
}

impl<T: ?Sized> SyncUnsafeCell<T> {
    pub const fn get(&self) -> *mut T;
    pub const fn get_mut(&mut self) -> &mut T;
    pub const fn raw_get(this: *const Self) -> *mut T;
}

impl<T> From<T> for SyncUnsafeCell<T>;
impl<T: Default> Default for SyncUnsafeCell<T>;
impl<T: ?Sized> Debug for SyncUnsafeCell<T>;

Steps / History

• Implementation: Add SyncUnsafeCell. #95438
• Final comment period (FCP)
• Stabilization PR

Unresolved Questions

• Name: SyncUnsafeCell? RacyUnsafeCell? RacyCell? UnsafeSyncCell? ..?
• Should it be Sync only if T is Sync, or should it unconditionally be Sync?
• Alternative: AssertThreadSafe (name TBD) – a more general API for lifting conservative !Send or !Sync implementations libs-team#322
• Partial alternative: Allow UnsafeCell in shared statics #152540

[bstrie]

The arguments in favor of having such a type are contained in the (very long) thread at #53639 .

TL;DR: static mut makes it way too easy to get a mutable reference to a static, which is wildly unsound in the face of concurrency, and having users write their own versions of this type is very error prone (see #53639 (comment) for one example). This is an unsafe pattern that should be encapsulated by the stdlib, and might someday lead to the ability to deprecate static mut.

[josephlr]

This would be a great addition to the standard library. Most OS-level crates I've worked on eventually have to use static mut or implement an abstraction like this if they need to have static mutable memory (such as a bootstrap stack or GDT). One of the downsides of static mut is that doing pointer math with addresses to static mut variables has to be marked unsafe. Even addr_of!(STATIC_VAR) needs an unsafe annotation.

This abstractions makes it so raw pointer math would be safe, while use of the pointee to would be unsafe.

A concrete example is from @phil-opp's blog "Writing an OS in Rust" where static mut is used for the TSS Double Fault stack.

[Pointerbender]

Unresolved Question: Should it be Sync only if T is Sync, or should it unconditionally be Sync?

I think the former might be the best, if it's unconditionally Sync then it would be possible to wrap !Send types (e.g. Rc or thread local storage values) in it and make it safe to share these between threads. At the very least it should be Sync if T is Send.

[Raekye]

• Should it be Sync only if T is Sync, or should it unconditionally be Sync?

Conceptually, I think T should be required to be Sync. And while we're at it, shouldn't it also require T: Send at the very least, as Pointerbender mentioned?

In practice, I commonly deal with types containing raw pointers (coming from bindgen), which are (very un-ergonomically) !Send and !Sync. So unfortunately, I'll have to continue using my own implementation which is unconditionally Sync, because of raw pointers being unconditionally !Send and !Sync.

[traviscross]

There has been previous discussion that if a new UnsafeCell type were to be added, that it should be both Sync and Copy. Does that have any bearing on this issue?

In particular, it seems as though stabilizing this without a Copy impl may complicate or preclude adding one later for the reasons that have made #55207 and #25053 challenging ¹. If we were to want a version of UnsafeCell that implements Copy, is it better to have both SyncUnsafeCell and CopyUnsafeCell or better to have one RawUnsafeCell (a.k.a. (farcically) VeryUnsafeCell, SuperUnsafeCell, etc.) that implements both and from which all others can be derived?

As background:

@SimonSapin proposed this primitive here: #55207 (comment) #53639 (comment)

@aturon discussed adding a separate Copy version of UnsafeCell here: #25053 (comment)

@RalfJung analyzed the situation in some detail: #25053 (comment) #55207 (comment)

cc #55207 #25053 @jamesmunns

Footnotes

1. That is, UnsafeCell could have been Copy originally, but adding the impl now is hard, just as UnsafeCell could have been Sync originally, but at this point we're instead adding a new struct. ↩

[jamesmunns]

In general, in my uses of UnsafeCell-made-sync, which are typically data structures that I am managing thread safety manually, e.g. for thread safe queues or allocators or such, I very much would not want it to be Copy. I can hunt down some concrete examples if it would be helpful, but I have a lot of UnsafeCell<MaybeUninit<T>>s in my code for reasons.

I'm not certain if the proposal was something like:

impl<T: Copy> Copy for NewUnsafeCell<T> {}

Which might be more palatable, but it would probably make me nervous that I might ACCIDENTALLY make my types copy, when I really don't want them to be, for example if I happened to use some T: Copy data in my structures.

I think a lot of mine (and maybe others?) desire for a Copy version of UnsafeCell has been abated by the fact that UnsafeCell::new() is now const, and Mara's "Const Single" pattern works, e.g.

const SINGLE: UnsafeCell<NonCopyType> = UnsafeCell::new(NonCopyType::const_new());
let arr: [UnsafeCell<NonCopyType>; 64] = [SINGLE; 64];

There might be others who have that desire! But you pinged me, and I can't say I've ever wanted it, outside of initializers, where the "Const Single" pattern works reasonably well, even if it isn't completely widely known.

Edit:

As a very quick example, bbqueue uses an UnsafeCell<MaybeUninit<[u8; N]>> for a backing storage.

c.f.: https://github.com/jamesmunns/bbqueue/blob/f73423c0b1c5fe04723e5b5bd57d1a44ff106473/core/src/bbbuffer.rs#L22-L54

I feel like it would be a footgun to make this structure Copy, even if MaybeUninit<[u8; N]> could be, and I think it would, given that [u8; N] is Copy.

[SimonSapin]

It looks like I did write a few years ago about a possible SomethingUnsafeCell that would both Sync and Copy, but now I agree with @jamesmunns that it’s probably not a good idea.

SyncUnsafeCell exists mostly to use in static to replace static mut where unintentional accesses can cause UB, whereas always manipulating raw pointers makes it easier to see where they are dereferenced. Making it also Copy would reintroduce that risk of unintentional (and not necessarily properly synchronized) access to a shared static.

[jamesmunns]

Oh. I just now realized I opened #55207 in 2018, which proposed to make UnsafeCell Copy/Clone (oh how time flies!)

As I mentioned downthread: #55207 (comment), I definitely agree that my original goal was mainly around const/array initialization, which is solved by (better IMO) by #49147.

[RalfJung]

Which might be more palatable, but it would probably make me nervous that I might ACCIDENTALLY make my types copy, when I really don't want them to be, for example if I happened to use some T: Copy data in my structures.

That would not happen. You still have to add derive(Copy) to your type, which of course you won't.

I really don't understand the problem here.

[jamesmunns]

In the example I gave, MaybeUninit<[u8; 64]> is Copy because [u8; 64] is Copy, and MaybeUninit<T> is Copy when T is.

Therefore without adding derive(Copy), SuperUnsafeCell<MaybeUninit<[u8; 64]>> is now Copy, which was not previously the case for UnsafeCell<MaybeUninit<[u8; 64]>>, which I might never want to be copy, because it serves as an inner-mutability data store, and any unintentional copies would be a logic error.

[RalfJung]

I assumed you were talking about a type you defined yourself.

Interior mutability data stores can be perfectly fine with Copy (making Cell: Copy would be sound), so it seems like a bad idea to disallow this for everyone and make it outright impossible to have an interior mutable type that is Copy. That's just a fundamental expressiveness gap that Rust currently has, and not for any good reason IMO.

[jamesmunns]

That's fair. However some of my worst bugs when working with unsafe are when I accidentally create a copy of some data that happens to be Copy (e.g. accidentally copying to the stack when trying to reborrow from a pointer), then returning a reference to a local rather than the non-local pointer. Since UnsafeCell is likely to be used in pointer form (e.g. through the .get() interface, using it with any T: Copy data makes me wary.

This is probably a niche issue (and almost certainly only a personal experience report), but that's my $0.02. If there are other compelling use cases, I'll just have to be more careful in the future, or stick with the old UnsafeCell with a manual Sync impl.