tokio-util: enable loom testing for cancellation token

[benjaminran]

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.

closes #7271

[benjaminran]

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]

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]

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

[ADD-SP]

Sorry for the late review.

[ADD-SP]

Suggested change

	run: cargo test --lib --release --features full -- --nocapture sync::tests
	run: cargo test --lib --release --features full -- --nocapture

tokio/tokio-util/src/either.rs

Lines 215 to 236 in 9255d96

	#[cfg(test)]
	mod tests {
	use super::*;
	use tokio::io::{repeat, AsyncReadExt, Repeat};
	use tokio_stream::{once, Once, StreamExt};
	#[tokio::test]
	async fn either_is_stream() {
	let mut either: Either<Once<u32>, Once<u32>> = Either::Left(once(1));
	assert_eq!(Some(1u32), either.next().await);
	}
	#[tokio::test]
	async fn either_is_async_read() {
	let mut buffer = [0; 3];
	let mut either: Either<Repeat, Repeat> = Either::Right(repeat(0b101));
	either.read_exact(&mut buffer).await.unwrap();
	assert_eq!(buffer, [0b101, 0b101, 0b101]);
	}
	}

It should be ok to exclude these tests from loom, and then run all unit tests using loom.

[ADD-SP]

Suggested change

	name: loom tokio-util::sync
	# base_ref is null when it's not a pull request
	if: github.repository_owner == 'tokio-rs' && (contains(github.event.pull_request.labels.*.name, 'R-loom-util-sync') || (github.base_ref == null))
	name: loom tokio-util::sync
	# base_ref is null when it's not a pull request
	if: github.repository_owner == 'tokio-rs' && (contains(github.event.pull_request.labels.*.name, 'R-loom-util') || (github.base_ref == null))

Since there is no many loom tests in tokio-util, it should be ok to enable all tests on CI for now.

[ADD-SP]

Suggested change

	- tokio-util/src/sync/*
	- tokio-util/src/sync/**/*
	- tokio-util/src/*
	- tokio-util/src/**/*

See #7644 (comment).