Add SyncUnsafeCell.

[m-ou-se]

This adds SyncUnsafeCell, which is just UnsafeCell except it implements Sync.

This was first proposed under the name RacyUnsafeCell here: #53639 (comment) and here: #53639 (comment) and here: #53639 (comment)

It allows you to create an UnsafeCell that is Sync without having to wrap it in a struct first (and then implement Sync for that struct).

E.g. static X: SyncUnsafeCell<i32>. Using a regular UnsafeCell as static is not possible, because it isn't Sync. We have a language workaround for it called static mut, but it's nice to be able to use the proper type for such unsafety instead.

It also makes implementing synchronization primitives based on unsafe cells slightly less verbose, because by using SyncUnsafeCell for UnsafeCells that are shared between threads, you don't need a separate impl<..> Sync for ... Using this type also clearly documents that the cell is expected to be accessed from multiple threads.

[rust-highfive]

r? @scottmcm

(rust-highfive has picked a reviewer for you, use r? to override)

[Aaron1011]

Bikeshed: I'm a little concerned that the name SyncUnsafeCell implies that it makes something Sync (e.g. introduced some kind of locking or indirection), rather than asserting that all of the uses of the obtained *mut T are consistent with the type being Sync.

[bstrie]

I'm settling in for a long bikeshed. EvenUnsaferCell, anyone?

[m-ou-se]

Bikeshed: I'm a little concerned that the name SyncUnsafeCell implies that it makes something Sync (e.g. introduced some kind of locking or indirection), rather than asserting that all of the uses of the obtained *mut T are consistent with the type being Sync.

Yeah, that's a valid point. There's a few alternative names on the tracking issue that we can bikeshed over. I was thinking of UnsafeSyncCell just to put the word Unsafe first, but it's also nice to keep UnsafeCell together to make it clear it's really just an UnsafeCell.

[bstrie]

I will say that this is a recurring problem with naming, where we have both Sync and non-Sync versions of some type and we need to figure out how to express the difference. Maybe there's a broader solution?

[BurntSushi]

I also like having Unsafe and Cell adjacent, as in, UnsafeCell should appear in the name.

I think I like RacyUnsafeCell more than SyncUnsafeCell, but not a huge fan of either to be honest.

🤔

[bstrie]

I think we can probably at least agree that RacyUnsafeCell would be better than just RacyCell, since it explicitly evokes UnsafeCell.

[m-ou-se]

I should note that I'm not sending this PR as 'the solution' to #53639. This type stands on its own, and might or might not be part of a proper solution for deprecating static mut. I think this type has value outside of statics.

[bstrie]

Can you elaborate on how else one might find this useful? Some examples of suggested use cases would be nice to have in the doc comment.

[m-ou-se]

I wanted to show someone a very simple example of synchronization through an AtomicBool like this:

let value = UnsafeCell::new(0);
let ready = AtomicBool::new(false);

// thread 1:
unsafe { *value.get() = 123 };
ready.store(true, Release);

// thread 2:
if ready.load(Acquire) {
    dbg!(unsafe { *value.get() });
}

And then realized I couldn't really give that example without defining my own type first, because the only way to express it's okay for my UnsafeCell to be shared between threads is by wrapping it in my own type and adding an unsafe impl Sync for it.

More generally, I think it'd be nice to use it in e.g. an RwLock<T> implementation, where using an SyncUnsafeCell clearly documents that the unsafe cell is meant to be shared between threads, and that we're responsible for adding synchronization to that.

[jamesmunns]

(Edited to provide full context)

I've found myself needing something like SyncUnsafeCell when working with hardware.

In the Atmel SAME70 microcontroller, the hardware uses DMA (direct memory access) in order to send and receive ethernet frames. In order to do that (messy PoC code here), the process looks (roughly) like this:

• The software prepares chunks of memory to use as buffers. These are used to contain the body of the ethernet frames
• The software provides "Buffer Descriptors", which act as a 'handle' for each buffer. These contain the address and some status bits for each of the buffers
• The software gives the hardware the address of the Buffer Descriptors, which the hardware uses for sending and receiving frames

In this case, the Buffer Descriptors act as the synchronization mechanism between the hardware and software. It is common in embedded rust to consider the hardware as a "separate thread", as it often executes asynchronously from the main software, in this case receiving and sending ethernet frames.

Additionally, it is considered "safest" to use static memory buffers when interacting with DMA, as memory on the stack can suffer from the same kind of "liveness" issues as you have with sharing stack data to multiple threads (basically: if you forget to 'turn off' the DMA before leaving the relevant stack frame: oops! memory corruption!).

For both the Buffer Descriptors and the Buffers themselves, they need to be UnsafeCells as the hardware can change the contents of either without interaction from the software. However, this means that I want both a static (for DMA stability/safety), as well as UnsafeCell (for correct behavior in the presence of multiple "thread" r/w access).

Safety is guaranteed by using a certain bit in the buffer descriptor in order to denote "ownership" of a given buffer. For example, when receiving frames:

• The software masks the lowest bit in a descriptor to let the hardware know that buffer is ready to be written to (by the hardware).
• The software then waits until the descriptor's lowest bit is set, which notes that the hardware has completed the transfer, and is ready for the software to read.
• This means I can safely give the user a reference to the buffer, knowing that the contents are valid, and will not be modified by the hardware
• Once the user is "done" with the buffer, and the handle is dropped, the descriptor is reset to clear the lowest bit, which signifies that access to the relevant buffer should not be allowed (it may be spuriously mutated at any time).

This is (admittedly) a long-winded example of the same situation/use-case that @m-ou-se described, using an atomicbool/bit to synchronize access, but this is a decently common pattern in the embedded world, and leads to a swath of unsafe impl {Sync|Send} definitions, which I think would better communicate intent with a dedicated SyncUnsafeCell type.

[joshtriplett]

This seems reasonable to me.

I do prefer Sync to Racy, since the latter suggests to me that the cell is inherently racy rather than just that it would be if you don't add proper synchronization around it.

I won't bikeshed SyncUnsafeCell vs UnsafeSyncCell; both seem reasonable, with different tradeoffs. On balance, SyncUnsafeCell seems slightly better to me.

Could the compiler potentially see through accesses to this, and treat them as compiler-level non-atomic (such as doing partial/tearable loads) or non-volatile (such as remembering the result of a previous read operation rather than re-reading), such that simple ways of protecting the cell aren't enough? How can someone do the equivalent of std::ptr::read_volatile with a SyncUnsafeCell, to prevent that? Would it potentially make sense to provide such an operation natively?

Given some answer to that question (even if the answer is "we don't need to do that, here's why"), I'd be happy to r+ this.

[m-ou-se]

Could the compiler potentially see through accesses to this, and treat them as compiler-level non-atomic (such as doing partial/tearable loads) or non-volatile (such as remembering the result of a previous read operation rather than re-reading), such that simple ways of protecting the cell aren't enough?

Whatever you use to provide the synchronization (e.g. a mutex or raw atomics) should provide everything that's needed to make sure things are read/written after/before the right moments. (E.g. memory ordering on atomics.)

How can someone do the equivalent of std::ptr::read_volatile with a SyncUnsafeCell

An unsafe cell only provides access through .get() which gives a pointer, so read_volatile and friends can direclty be used on that pointer, just like how every other interaction needs to go through that pointer.

All the use cases for SyncUnsafeCell just use UnsafeCell today, but have to be paired with a unsafe impl Sync for .., which means that they always need to be wrapped in another type and cannot be used directly. SyncUnsafeCell is a better way of documenting that the unsafe cell is meant to be shared between threads, instead of using a type that explicitly forbids that (UnsafeCell) and then manually overriding it (with unsafe impl Sync). It won't need anything other than what UnsafeCell already has. (Just .get().)

[joshtriplett]

An unsafe cell only provides access through .get() which gives a pointer, so read_volatile and friends can direclty be used on that pointer, just like how every other interaction needs to go through that pointer.

@bors r+

[bors]

📌 Commit f225808 has been approved by joshtriplett