mpmc: add loom test for enqueue/dequeue contention #618

0xllx0 · 2025-10-03T15:47:00Z

Adds a loom permutation test for mpmmc::Queue::enqueue contention across threads.

Related: #583
Co-authored-by: NODA Kai @nodakai
Co-authored-by: Sosthène Guédon @sosthene-nitrokey

0xllx0 · 2025-10-03T15:50:39Z

src/mpmc.rs

+    fn issue_583_enqueue_loom() {
+        const N: usize = 4;
+
+        loom::model(|| {
+            let q0 = loom::sync::Arc::new(Queue::<u8, N>::new());
+            for i in 0..N {
+                q0.enqueue(i as u8).expect("new enqueue");
+            }
+            eprintln!("start!");
+
+            let q1 = q0.clone();
+            loom::thread::spawn(move || {
+                for k in 0..1000_000 {
+                    if let Some(v) = q0.dequeue() {
+                        q0.enqueue(v)
+                            .unwrap_or_else(|v| panic!("{k}: q0 -> q0: {v}, {:?}", to_vec(&*q0)));
+                    }
+                }
+            });
+
+            loom::thread::spawn(move || {
+                for k in 0..1000_000 {
+                    if let Some(v) = q1.dequeue() {
+                        q1.enqueue(v)
+                            .unwrap_or_else(|v| panic!("{k}: q0 -> q0: {v}, {:?}", to_vec(&*q1)));
+                    }
+                }
+            });
+        });
+    }


This loom test passes, but is also not a direct replication of the failing test harnesses. There is currently no loom::thread::scope equivalent of std::thread::scope.

Edit: there was some work to add loom::thread::scope in loom/!312, but the author stopped working on it in May 2024.

Interestingly, adding a test using loom::thread::scope from the linked PR also passes.

sgued · 2025-10-05T19:29:18Z

Thanks for working on fixing #583

Please add loom first and change the implementation in a separate PR, this will make it much easier to make sur that the tests are working as intended, and will make review much easier.

Regarding why the loom tests pass: you're misusing loom. You need to add a #[cfg(not(loom))] to all use core::sync::atomic* and a [cfg(loom)] that instead imports the atomics from loom. You then need to pass a --cfg loom flag to the compiler when you run the loom tests (which should be #[cfg(loom)] too)

Here loom doesn't see any of the atomic operation so assumes all went fine.

Also please use stable loom . Using Box::leak or Arc is fine here.

0xllx0 · 2025-10-06T05:16:29Z

Thanks for working on fixing #583

You're welcome <3

Please add loom first and change the implementation in a separate PR, this will make it much easier to make sur that the tests are working as intended, and will make review much easier.

Sounds good, I just wanted to add the alternate implementation here first to make the comparison easier.

I'll create a separate PR for the alternate implementation.

I'll address the loom tests below.

Regarding why the loom tests pass: you're misusing loom. You need to add a #[cfg(not(loom))] to all use core::sync::atomic* and a [cfg(loom)] that instead imports the atomics from loom. You then need to pass a --cfg loom flag to the compiler when you run the loom tests (which should be #[cfg(loom)] too)

Here loom doesn't see any of the atomic operation so assumes all went fine.

Also please use stable loom . Using Box::leak or Arc is fine here.

This definitely sounds like it requires a separate PR. I'm new to the loom library, so not surprising that I'm using it wrong.

Since the failing conditions can be reached by normal unit-tests, I'll probably wait to implement the loom tests until after we decide on the alternate implementation PR.

Edit: after a small experiment using a feature-gate for use loom::sync::atomic in mpmc, it will require some ugly feature-gates to make the queue constructors non-const for loom builds. May end up worthwhile to support permutation tests.

The loom changes for the current mpmc::Queue are not so bad, but running the tests give the error:

thread 'mpmc::tests::issue_583_enqueue_loom' panicked at /home/user/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/loom-0.7.2/src/rt/path.rs:247:13:
Model exceeded maximum number of branches. This is often caused by an algorithm requiring the processor to make progress, e.g. spin locks.

This probably means the loom tests won't tell us anything, since both implementations rely on versions of spin-locks. I'm not going to go through the refactoring effort of making loom primitives work with the crossbeam implementation if it's throwing these kind of errors with the current mpmc implementation. The refactors for the crossbeam implementation are much more involved.

zeenix · 2025-10-06T11:52:44Z

Also please use stable loom . Using Box::leak or Arc is fine here.

This definitely sounds like it requires a separate PR

Why? If it's just changes to unmerged changes, then it can (and should be) the same PR. Otherwise, it only creates unnecessary noise. Just modify your commits (if you have to) and force push to your branch.

0xllx0 · 2025-10-06T13:58:47Z

Also please use stable loom . Using Box::leak or Arc is fine here.

This definitely sounds like it requires a separate PR

Why? If it's just changes to unmerged changes, then it can (and should be) the same PR. Otherwise, it only creates unnecessary noise. Just modify your commits (if you have to) and force push to your branch.

What I meant was that the loom stuff should be separate from the fix PR, which I've opened in #619. The loom tests fail for the existing implementation, and likely the port of the crossbeam implementation. They fail with an error about spin-locks:

thread 'mpmc::tests::issue_583_enqueue_loom' panicked at /home/user/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/loom-0.7.2/src/rt/path.rs:247:13:
Model exceeded maximum number of branches. This is often caused by an algorithm requiring the processor to make progress, e.g. spin locks.

Adds a `loom` permutation test for `mpmmc::Queue::enqueue/dequeue` contention across threads. Related: rust-embedded#583 Co-authored-by: NODA Kai <https://github.com/nodakai> Co-authored-by: Sosthène Guédon <https://github.com/sosthene-nitrokey>

0xllx0 force-pushed the test/issue-583 branch from c4f9b25 to 9895080 Compare October 3, 2025 15:48

0xllx0 commented Oct 3, 2025

View reviewed changes

0xllx0 mentioned this pull request Oct 3, 2025

MpMcQueue can fail to push even when it's not full under contention #583

Open

0xllx0 force-pushed the test/issue-583 branch from 9fc8818 to b7f9559 Compare October 4, 2025 04:06

0xllx0 force-pushed the test/issue-583 branch from b7f9559 to c1abf02 Compare October 6, 2025 14:45

0xllx0 force-pushed the test/issue-583 branch from c1abf02 to 9ba3eff Compare October 6, 2025 14:47

ci: add runner for loom tests

591eef7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

mpmc: add loom test for enqueue/dequeue contention #618

mpmc: add loom test for enqueue/dequeue contention #618

0xllx0 commented Oct 3, 2025

Uh oh!

0xllx0 Oct 3, 2025 •

edited

Loading

Uh oh!

0xllx0 Oct 3, 2025

Uh oh!

sgued commented Oct 5, 2025 •

edited

Loading

Uh oh!

0xllx0 commented Oct 6, 2025 •

edited

Loading

Uh oh!

zeenix commented Oct 6, 2025 •

edited

Loading

Uh oh!

0xllx0 commented Oct 6, 2025

Uh oh!

Uh oh!

mpmc: add loom test for enqueue/dequeue contention #618

Are you sure you want to change the base?

mpmc: add loom test for enqueue/dequeue contention #618

Conversation

0xllx0 commented Oct 3, 2025

Uh oh!

0xllx0 Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xllx0 Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

sgued commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xllx0 commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeenix commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xllx0 commented Oct 6, 2025

Uh oh!

Uh oh!

0xllx0 Oct 3, 2025 •

edited

Loading

sgued commented Oct 5, 2025 •

edited

Loading

0xllx0 commented Oct 6, 2025 •

edited

Loading

zeenix commented Oct 6, 2025 •

edited

Loading