-
Notifications
You must be signed in to change notification settings - Fork 17
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
Why UnsafeCell in Mutex? #52
Comments
We discussed this one at today's Embedded WG meeting. We're still thinking about it… Should see more comments here shortly. |
I don't know if the But I wondered if it does make a difference. After all,
I looked at the third case in compiler explorer, and indeed, mutex1 (without the UnsafeCell) generates slightly shorter code than mutex2 (with the UnsafeCell): https://godbolt.org/z/bo5KfsnWG So, assuming that it is sound to remove the |
As suggested by @thalesfragoso, I also tried wrapping |
I am not an expert in compiler internals, not even close. However, I want to summarize all possible usages of Let's explore all the details of a simpler implementation of 1. If
|
The core idea of a mutex is to make
!Sync
typesSync
. This is exactly what this line doescritical-section/src/mutex.rs
Line 191 in 3a328a8
!Sync
type == cannot function properly when used concurrently behind shared references.To fulfill the safety contract we must guarantee that all shared access to the inner type is synchronized. From the perspective of critical sections, fulfillment of the safety contract depends only on proper handling of lifetimes, because while owning a valid
CriticalSection<'cs>
, any shared access to inner type is safe.There is no actual benifit to using
UnsafeCell
here, and the only requirement is to ensure that shared access to the inner type bound by'cs
lifetime.As I ran some tests, it seems like additional
UnsafeCell
layer does not cause any performance regressions. But if we change it to justT
, curious people like me will have fewer questions regarding necessity of such wrappers :)The text was updated successfully, but these errors were encountered: