Skip to content

Commit

Permalink
Reduce loom test-threads to 1
Browse files Browse the repository at this point in the history
This seems to be the only reliable workaround for
<tokio-rs/loom#316>

On the bright side this lets us increase the number of threads tested in
`test_reuse_keys_after_thread_exit`
  • Loading branch information
sporksmith committed Jul 3, 2023
1 parent ed406f5 commit 719ba6f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/sanitizers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ jobs:
# If this ever gets too slow, we can consider reducing `LOOM_MAX_PREEMPTIONS`
# to substantially speed it up, at the cost of losing some coverage of possible
# execution paths.
#
# We use `--test-threads 1` as a workaround for
# <https://github.com/tokio-rs/loom/issues/316>
RUST_BACKTRACE=1 \
RUSTFLAGS="--cfg loom" \
cargo test -p vasi-sync
cargo test -p vasi-sync -- --test-threads 1
11 changes: 6 additions & 5 deletions src/lib/vasi-sync/tests/atomic-tls-map-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,16 @@ mod atomic_tls_map_tests {
})
}

// This test seems to cause mysterious crashes in loom itself when running tests
// with `--test-threads` > 1. Reducing max_preemptions or NTHREADS has
// temporarily fixed it in the past, and then adding or changing *some other
// test* brings the failure back.
// <https://github.com/tokio-rs/loom/issues/316>
#[test]
fn test_reuse_keys_after_thread_exit() {
sync::model_with_max_preemptions(2, || {
// NTHREADS > 2 *crashes* loom. 2 should be sufficient for this test, though.
// e.g. test_get_and_remove_threaded should already cover concurrent insert +
// remove + lookup.
// <https://github.com/tokio-rs/loom/issues/316>
#[cfg(loom)]
const NTHREADS: usize = 2;
const NTHREADS: usize = 3;
#[cfg(not(loom))]
const NTHREADS: usize = 100;

Expand Down

0 comments on commit 719ba6f

Please sign in to comment.