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

Allow unguarded access to an RwLock's inner value using unsafe #531

Open
jplatte opened this issue Feb 1, 2025 · 6 comments
Open

Allow unguarded access to an RwLock's inner value using unsafe #531

jplatte opened this issue Feb 1, 2025 · 6 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@jplatte
Copy link

jplatte commented Feb 1, 2025

Proposal

Problem statement

I would like to be able to avoid pointer casts for unguarded accesses of the inner value of an RwLock, similar to RefCells try_borrow_unguarded.

Motivating examples or use cases

I am the maintainer of the readlock crate.

About this crate

At its core, this crate provides a a few types that can act as a replacement for Arc<RwLock<T>> if only a single clone of that Arc is ever used for writing. This is done by introducing two wrapper types around Arc<RwLock<T>>, Shared<T> which can be used for writing but cannot be cloned, and ReadLock<T> which can be obtained from Shared<T> and cloned, but only used for reading. Splitting the capabilities into two types like this reduces the potential for deadlock bugs for code that fits this pattern, and can allow downstream crates to expose a handle to some internally-managed state in a read-only way without introducing custom lock types.

Its type Shared<T> has shared ownership of an RwLock<T>, but guarantees that no other reference to that RwLock ever locks it for writing. It also requires &mut self for write-locking the inner value. Due to this, it can implement Deref<Target = T>.

To implement Deref<Target = T>, I currently call .read() on the inner RwLock, then (unless there's a PoisonError) dereference the RwLockReadGuard and unsafely lifetime-extend the resulting reference using two casts (to raw pointer and back).
If I could instead call an unsafe fn on the RwLock with clear safety requirements that gives me &T directly w/o going through RwLockReadGuard, it would be a lot easier for me to prove my code sound - I am pretty certain the concept is sound (but happy to be proven wrong), but less certain about the current implementation being sound.

Further, and this may deserve to be split into its own ACP, but it seems worth pointing out here: It should also be sound here, and benefit performance, if the new API also allowed unchecked access to the inner value, i.e. if it did not require any reads the reader / writer count of the RwLock.

Solution sketch

Honestly, I don't know. I guess a direct equivalent of RefCell::try_borrow_unguarded would be possible and solve my bigger problem around soundness, but it would be really nice to get the performance benefits of unchecked access as well.

Alternatives

As described above, I already have an implementation of the sort of thing this would enable, I'm just not sure whether it's sound. See the readlock crate's source if you want to see the details.

@jplatte jplatte added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Feb 1, 2025
@Amanieu
Copy link
Member

Amanieu commented Feb 4, 2025

rust-lang/rust#133572 is in the process of adding UniqueArc which gives you mutable access to data behind an Arc as long as it hasn't been cloned yet. Would this be a better solution to your problem?

@jplatte
Copy link
Author

jplatte commented Feb 4, 2025

That's very nice, but unrelated to the feature I want.

Let me try to explain it another way: In my case there are multiple clones of the Arc<RwLock<T>>, but one of the clones is statically guaranteed to be the only "special" one which allows writing, and writing through that not only takes a write lock, it does so from an &mut self method. Because of that, when I call an &self method on this special instance, I know statically that the inner value is not write-locked (but potentially read-locked), which means it's safe to access it as &T (no RwLockReadGuard, preferably not even checking for existing lock guard holders), there's just no nice API for it yet.

@Amanieu
Copy link
Member

Amanieu commented Feb 4, 2025

Have you considered using a RwLock<()> with a separate UnsafeCell<T> and then managing the synchronization manually? This is effectively what RwLock is doing internally.

@jplatte
Copy link
Author

jplatte commented Feb 4, 2025

I didn't previously but after thinking a bit, this seems worse than what I have now for three reasons:

  • I need to add at least two unsafe impl Syncs then, and make sure they are correct
  • I go from one unsafe operation to a lot of them: read-locking (from one of the non-special arc clones) and write-locking are currently entirely safe, but would no longer be
  • I lose functionality / break my API because I can no longer use std's lock guards

@Amanieu
Copy link
Member

Amanieu commented Feb 11, 2025

One solution proposed in the libs-api meeting was for RwLock and Mutex to expose a raw pointer to the internal UnsafeCell via a data_ptr method. Would this be OK for your use case?

@jplatte
Copy link
Author

jplatte commented Feb 11, 2025

Yes, that would be perfect for my use case! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

2 participants