How can I enable `--cfg=crossbeam_loom` in a dependency?

[davidbarsky]

Hello! I'm trying to enable Loom in https://github.com/salsa-rs/salsa/. Since Salsa uses crossbeam (namely, SegQueue and AtomicCell) to write some fiddly, unsafe code, I wanted to also enable crossbeam's crossbeam_loom feature flag. Unfortunately, I've been struggling to enable it via RUSTFLAGS. For instance, a env RUSTFLAGS="--cfg=crossbeam_loom" cargo check doesn't seem to enable correctly enable crossbeam_util's Loom flag, failing with with errors along the lines of:

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:49:24
   |
49 |         pub(crate) use loom::hint::spin_loop;
   |                        ^^^^ use of undeclared crate or module `loom`

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:53:28
   |
53 |             pub(crate) use loom::sync::atomic::{
   |                            ^^^^ use of undeclared crate or module `loom`

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:63:28
   |
63 |             pub(crate) use loom::sync::atomic::fence as compiler_fence;
   |                            ^^^^ use of undeclared crate or module `loom`

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:65:24
   |
65 |         pub(crate) use loom::sync::{Arc, Condvar, Mutex};
   |                        ^^^^ use of undeclared crate or module `loom`

How can I enable crossbeam's Loom flag? Is it... even possible?

[taiki-e]

Loom is cfg-ed optional dependency, so you have to enable both crossbeam_loom cfg and loom feature.

crossbeam/crossbeam-utils/Cargo.toml

Lines 34 to 39 in 75d24e0

	# Enable the use of loom for concurrency testing.
	#
	# NOTE: This feature is outside of the normal semver guarantees and minor or
	# patch versions of crossbeam may make breaking changes to them at any time.
	[target.'cfg(crossbeam_loom)'.dependencies]
	loom = { version = "0.7.1", optional = true }

This is a bit odd, but unfortunately necessary to address the problem that the way cargo and crates.io handle cfg-ed dependencies is not very good, confuses users, and some people complain when new dependency appears in Cargo.lock (even if it never actually builds).
(e.g., #487 (comment), #665, tokio-rs/bytes#411, tokio-rs/bytes#400, etc.)
See also #666.

[davidbarsky]

Ah, thanks for your answer! The following:

[target.'cfg(crossbeam_loom)'.dependencies]
crossbeam-utils = { version = "0.8", features = ["loom"] }
crossbeam-epoch = { version = "0.9", features = ["loom"] }

...with a RUSTFLAGS="--cfg=crossbeam_loom" cargo check mostly does the trick. The only remaining issue is AtomicCell not being available under Loom. I can open an new issue for this, but would you accept a PR that partially backs out #787 and documents that the size might not be the same under Loom, or would that break some non-trivial things in crossbeam?

[taiki-e]

The issue is that they do not work because they have different structures to begin with. The implementation before #787 is completely broken.

AFAIK, what could be implemented is a fallback implementation, such as the one referenced by the review comment in that PR, or one that just uses Mutex internally. (Or change something on the Loom side.)

So, for now, I would suggest using Mutex or RwLock instead of AtomicCell when using Loom.

[davidbarsky]

The implementation before #787 is completely broken.

Ah, I didn't realize that was the case. Good to know.

AFAIK, what could be implemented is a fallback implementation, such as the one referenced by the review comment in that PR, or one that just uses Mutex internally. (Or change something on the Loom side.)
So, for now, I would suggest using Mutex or RwLock instead of AtomicCell when using Loom.

I can use a lock in the short-term when consuming crossbeam. If you'd like, I'd also be happy to have crossbeam itself fallback to Mutex internally, as SeqQueue uses AtomicCell and I think it'd be nice to extend Loom testing to crossbeam's queues.

[taiki-e]

as SeqQueue uses AtomicCell

If you are talking about crossbeam_queue::SegQueue, crossbeam_queue doesn't use AtomicCell at all. (In crossbeam, only crossbeam_channel::tick uses AtomicCell)

The reason why crossbeam_queue does not currently support Loom is for a completely different reason. When I tried it before, unbounded queue panicked with odd errors like tokio-rs/loom#283.