cargo test 'test-threads' uses more threads than specified

[jweinst1]

Summary

So I have noticed that cargo test actually uses more threads than is actually specified, and actually spawns a new thread for every test, as opposed to re-using threads in a pool. This is in many ways a bug because some tests the depend on thread local storage cannot be run in concurrent mode.

Reproduction Steps

1. Create a fresh cargo project with a library.
2. Create a file called tlocal.rs , and list it in the lib.rs as a module.
3. Use the following code to create a very simple thread local storage:

use std::sync::atomic::{AtomicUsize, Ordering};
use std::thread;

static THREAD_CNTR:AtomicUsize = AtomicUsize::new(0);

thread_local!(static TH_ID:usize = THREAD_CNTR.fetch_add(1, Ordering::SeqCst));

pub fn tid() -> usize {
    TH_ID.with(|x| { *x })
}

4. Write a test module in the same file, such as

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn tid_works() {
    	assert!(tid() < 2);
    }

    #[test]
    fn tid_works2() {
    	assert!(tid() < 2);
    }

    #[test]
    fn tid_works3() {
    	assert!(tid() < 2);
    }
}

5. Run cargo test -- --test-threads=2

I expected to see this happen: The tests pass.

Instead, this happened: The test fail with these errors:

---- tlocal::tests::tid_works stdout ----
thread 'tlocal::tests::tid_works' panicked at 'assertion failed: tid() < 2', src/tlocal.rs:62:9

---- tlocal::tests::tid_works2 stdout ----
thread 'tlocal::tests::tid_works2' panicked at 'assertion failed: tid() < 2', src/tlocal.rs:67:9

---- tlocal::tests::tid_works3 stdout ----
thread 'tlocal::tests::tid_works3' panicked at 'assertion failed: tid() < 2', src/tlocal.rs:72:9

Meta

rustc --version --verbose:

% rustc --version --verbose      
rustc 1.52.1 (9bc8c42bb 2021-05-09)
binary: rustc
commit-hash: 9bc8c42bb2f19e745a63f3445f1ac248fb015e53
commit-date: 2021-05-09
host: x86_64-apple-darwin
release: 1.52.1
LLVM version: 12.0.0

Problem

This is a bug because more than the stated number of threads are actually being used to run the tests. Ideally, this should either be clarified in the docs, or a way of running tests in a thread pool should be provided.

[epage]

The framing I use for --test-threads is that it is for resource management (how many tests can run in parallel) and not for strictly controlling how many threads exist.

For some background on where we are at

1. libtest: Use deterministic HashMap, avoid spawning thread if there is no concurrency #56243
2. "Cargo test" uses a thread pool in 1.33.0 #58907
3. libtest: run all tests in their own thread, if supported by the host #103681
4. Disable thread support in libtest if RUST_TEST_THREADS=1 #107396

From #107396 (comment)

I don't think we should make this behaviour implicit on RUST_TEST_THREADS=1. We might want another environment variable or a specical attribute to run a test on the main thread for example, but that's a design question separate from this PR, that will require an ACP and should probably be designed together with the testing team.

Based on the FCP for #107396, I'm going to close this as this seems to be intended behavior.