Skip to content

Conversation

benjaminran
Copy link

@benjaminran benjaminran commented Sep 23, 2025

Motivation

This enables the tokio-util loom tests to run in CI.

Solution

This starts triggering the existing tokio_util::sync looms tests (currently just loom_cancellation_token.rs) according to label R-loom-util-sync / code changes to tokio_util::sync::*.

Conditionally skipped defining a few modules/types (tokio_util::net, tokio_util::udp, some of tokio_stream::wrappers::*) when cfg(loom) since they depend on tokio::net etc. which are likewise conditionally skipped.

@benjaminran
Copy link
Author

If the R-loom-util-sync / tokio_util::sync scoping sounds good, could somebody with permissions add the R-loom-util-sync label to the repo?

I think this is close to ready, but TBD what the CI will raise, and I am seeing the last 3 loom_cancellation_token.rs tests failing locally due to what may be a real concurrency bug (if so, pretty cool). I'll try to see tomorrow if it's a simple enough issue that I can fix it here. If not then I'd just #[ignore] those tests and fix them separately - lmk how that sounds

@benjaminran benjaminran marked this pull request as draft September 24, 2025 08:33
@benjaminran
Copy link
Author

Re the 3 failing loom tests, loom actually seems to be incorrectly declaring a deadlock there. Looks like it's when a task1 does potential_parent.inner.try_lock() from within with_locked_node_and_parent as part of dropping a child token of the root token and a task2 does child.inner.lock() from within disconnect_children as part of dropping the root token. Really task1 is still runnable and would proceed to release its lock on the child, unblocking task2. Trace logs with some debug prints in https://gist.github.com/benjaminran/9c0d3b80be7e61dcae2735e065b33bf3.

tokio-rs/loom#376 sounds very related, but I tried running with the loom changes in tokio-rs/loom#383 and the issue didn't go away :(

@benjaminran
Copy link
Author

This is ready for review now - looks like enabling those 3 ignored tests will require a fix in loom

@benjaminran benjaminran marked this pull request as ready for review September 24, 2025 14:40
@ADD-SP ADD-SP added A-tokio-util Area: The tokio-util crate M-sync Module: tokio/sync labels Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-util Area: The tokio-util crate M-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants