Skip to content

Conversation

@martin-g
Copy link
Member

@martin-g martin-g commented Sep 7, 2025

Motivation

bd4ccae introduced a wrapper for the RwLock to get rid of poisoning aspects.

By mistake (?!) its try_read/write methods actually delegate to read/write() and this would lead to blocking

Solution

Delegate the to try_** methods of the delegate to avoid blocking.

@martin-g martin-g force-pushed the loom-parking-lot-try branch from 57772b9 to ffbc46b Compare September 7, 2025 20:09
bd4ccae introduced a wrapper for the
RwLock to get rid of poisoning aspects.

By mistake (?!) its try_read/write methods actually delegate to
read/write() and this would lead to blocking

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
@martin-g
Copy link
Member Author

martin-g commented Sep 7, 2025

@tglane Could you please review since you introduced it with #6807 ?

@ADD-SP ADD-SP added the A-tokio Area: The main tokio crate label Sep 8, 2025
@Darksonn
Copy link
Contributor

Darksonn commented Sep 8, 2025

Huh ... are these methods used anywhere?

@martin-g
Copy link
Member Author

martin-g commented Sep 8, 2025

are these methods used anywhere?

No idea.
I just noticed that they call their blocking versions and it looked suspicious.

@Darksonn
Copy link
Contributor

Darksonn commented Sep 8, 2025

Yes definitely it should be fixed.

Try removing them and see if it still compiles.

@tglane
Copy link
Contributor

tglane commented Sep 8, 2025

@tglane Could you please review since you introduced it with #6807 ?

Yep, that's definitely a copy/paste mistake. I cannot completely remember why I added those functions to the type (I guess because the Mutex implementation I took for inspiration had them, too). So they should either be removed or call the try_* methods of the inner type. Good catch!

@martin-g
Copy link
Member Author

martin-g commented Sep 9, 2025

Removing the try_** methods does not break the build:

diff --git i/tokio/src/loom/std/parking_lot.rs w/tokio/src/loom/std/parking_lot.rs
index 9367ae01..154c9607 100644
--- i/tokio/src/loom/std/parking_lot.rs
+++ w/tokio/src/loom/std/parking_lot.rs
@@ -100,21 +100,9 @@ impl<T> RwLock<T> {
         RwLockReadGuard(PhantomData, self.1.read())
     }

-    pub(crate) fn try_read(&self) -> Option<RwLockReadGuard<'_, T>> {
-        self.1
-            .try_read()
-            .map(|guard| RwLockReadGuard(PhantomData, guard))
-    }
-
     pub(crate) fn write(&self) -> RwLockWriteGuard<'_, T> {
         RwLockWriteGuard(PhantomData, self.1.write())
     }
-
-    pub(crate) fn try_write(&self) -> Option<RwLockWriteGuard<'_, T>> {
-        self.1
-            .try_write()
-            .map(|guard| RwLockWriteGuard(PhantomData, guard))
-    }
 }

cargo clean && cargo build --release && cargo test passes successfully!
I'm not sure how much more code in unused here.

@Darksonn Darksonn merged commit ac4c959 into tokio-rs:master Sep 9, 2025
94 checks passed
@martin-g martin-g deleted the loom-parking-lot-try branch September 9, 2025 07:39
@Darksonn Darksonn mentioned this pull request Oct 14, 2025
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants