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

sync: add a rwlock() method to owned RwLock guards #6418

Merged
merged 4 commits into from
Mar 24, 2024

Conversation

r3v2d0g
Copy link
Contributor

@r3v2d0g r3v2d0g commented Mar 20, 2024

Adds a rwlock() method returning a reference to the original Arc<RwLock> to the Owned* guards.

Motivation

Mutex guards and Semaphore permits already have similar methods.

Solution

A similar, rwlock(), method is added to the Owned* guards. The "normal" guards don't actually hold a reference to the original RwLock and I don't think that it can be added to them without changing these types' definitions.

@github-actions github-actions bot added the R-loom-sync Run loom sync tests on this PR label Mar 20, 2024
Adds a `rwlock()` method returning a reference to the original `Arc<RwLock>` to the `Owned*` guards.
@r3v2d0g r3v2d0g changed the title Add a rwlock() method to owned RwLock guards sync: add a rwlock() method to owned RwLock guards Mar 20, 2024
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Mar 20, 2024
@Darksonn
Copy link
Contributor

This is reasonable enough, but if we are adding it, then I would like us to be consistent and add it to all of the guards. I am okay with replacing the &Semaphore field with an &RwLock<T> field.

@r3v2d0g r3v2d0g marked this pull request as ready for review March 20, 2024 16:41
@r3v2d0g
Copy link
Contributor Author

r3v2d0g commented Mar 20, 2024

This is reasonable enough, but if we are adding it, then I would like us to be consistent and add it to all of the guards. I am okay with replacing the &Semaphore field with an &RwLock<T> field.

Although replacing the &Semaphore with an &RwLock<T> would work for RwLockWriteGuard because it is generic over the same T as the RwLock<T>, it wouldn't for RwLockMappedWriteGuard and RwLockReadGuard because both of these are generic over a type which might or might not be the same as the RwLock (RwLockWriteGuard<'a, T>::map returns RwLockMappedWriteGuard<'a, U> and RwLockReadGuard<'a, T>::map returns RwLockReadGuard<'a, U>).

The only way to make this work (AFAIK) would be to update RwLockMappedWriteGuard's signature from RwLockMappedWriteGuard<'a, U> to RwLockMappedWriteGuard<'a, T, U> where either T or U is the type wrapped by the RwLock, and to update RwLockReadGuard's signature to be similar to OwnedRwLockReadGuard, that is RwLockReadGuard<'a, T, U> where by default U = T.

Hope this all makes sense!

Copy link
Member

@mox692 mox692 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@mox692 mox692 merged commit 4565b81 into tokio-rs:master Mar 24, 2024
75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-sync Module: tokio/sync R-loom-sync Run loom sync tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants