Tests involving thread local var fails with Tarpaulin

[ggwpez]

Describe the bug
I have a minimal test that works with cargo test but fails with cargo tarpaulin.
cargo test seems to clear thread local variables before running each #[test], tarpaunlin does not; leading to different results.

How to Reproduce
Quite easy to reproduce:

use std::cell::RefCell;

thread_local! {
    static STICKY: RefCell<bool> = RefCell::new(false);
}

#[test]
fn tarpauline() {
    // STICKY must be false since we just started the test.
    assert!(!STICKY.with(|s| *s.borrow()));
    // Set to true.
    STICKY.with(|s| s.replace(true));
}

#[test]
fn tarpauline2() {
    // STICKY must be false since we just started the test.
    assert!(!STICKY.with(|s| *s.borrow())); // <- FAILS HERE
    // Set to true.
    STICKY.with(|s| s.replace(true));
}

The tests both work with cargo test, but one of them always fails with cargo tarpaulin, depending on the execution order.
error trace.txt

Expected behavior
Tests should not fail.

Versions
rustc 1.59.0-nightly (0b42deacc 2021-12-09
cargo 1.58.0-nightly (40dc28175 2021-12-06)
cargo-tarpaulin version: 0.18.5

@enthusiastmartin:
Maybe this is related to your issue #885, since I found it while working on Substrate.
The work-around in my case was to reset the thread local vars at the beginning of each test.

[xd009642]

Ah I can confirm it is fine on stable and doesn't work on nightly, so there's a decent chance it's the same issue as #885, thanks for providing this good minimal example 👍

[xd009642]

Also if you do the test with --test-threads=1 then it fails outside of tarpaulin, but cargo +nightly test does pass. I wonder if the overhead of tarpaulin means the tests have more of a delay before they start and end up on the same thread causing this 🤔

[ggwpez]

So this is not really a problem with tarpaulin but either:

• my test being wrong because it uses thread_local!
• the fact that cargo test produces different results depending on the --test-threads arg

[xd009642]

Well I would say having mutable global state in a test like that is wrong in a sense, say if the cargo test runner uses a threadpool (I haven't looked at the code in a while so not sure) it may be able to trigger this behaviour with enough of these tests to exhaust the entire pool. But yeah the --test-threads arg changing behaviour is something I would try to avoid in tests.

But, I'd still like to look into tarpaulin to figure out what causes the difference to emerge here even if for some users solving this may mask more intrinsic issues to their tests (but issues they aren't hitting with cargo test so some would say acceptable issues)

[xd009642]

So it looks like rustc test runner uses the scheduler affinity to work out max threads now, and tarpaulin had added something (years ago I think) to force it to a single core to attempt to mitigate a ptrace issue of some form. The ptrace issue was later fixed and I felt very strongly that affinity stuff was harmless and potentially a boon but now it will make tests run slower so I'm removing it. If you try this branch you should see it works for the example #894

I'm not adding the example to a test (as I would often do), and that's because for the reasons above I don't thing it's a actually a good test, and it working is largely dependent on the implementation of the test runner used.

[ggwpez]

If you try this branch you should see it works for the example #894

Thanks very much for the quick reaction! I will give it a shot.

[xd009642]

I went for an alternate solution of explicitly setting the test threads as changing affinity seemed to cause a segfault on another users PC. It's now merged into develop - I'll probably do a release this week just gonna see if there are any easy wins on other issues first 👀