Skip to content

Conversation

@japaric
Copy link
Member

@japaric japaric commented Apr 29, 2022

to catch false positives in newer version of scoped_threadpool

this change is required to prevent this snippet triggering a TSAN error (data race)

// scoped_threadpool = "0.1.9"

use scoped_threadpool::Pool;

fn main() {
    Pool::new(2).scoped(move |scope| {
        scope.execute(move || {});
        scope.execute(move || {});
    });
}

to catch false positives in newer version of scoped_threadpool

this change is required to prevent this snippet triggering a TSAN error (data race)
in debug & release mode

``` rust
// scoped_threadpool = "0.1.9"

use scoped_threadpool::Pool;

fn main() {
    Pool::new(2).scoped(move |scope| {
        scope.execute(move || {});
        scope.execute(move || {});
    });
}
```
@japaric
Copy link
Member Author

japaric commented Apr 29, 2022

the supressions seem to be too heavy handed.

this triggers a data race as expected (debug profile)

use std::thread;

static mut X: i32 = 0;

let t1 = thread::spawn(|| unsafe { ptr::addr_of_mut!(X).write_volatile(1) });
unsafe {
    ptr::addr_of_mut!(X).write_volatile(2);
}
t1.join().ok();
WARNING: ThreadSanitizer: data race (pid=45823)
(..)
Location is global 'foo::main::X::h5af7d740da9a743d' of size 4

with the above supressions, this does NOT trigger a data race

Pool::new(2).scoped(move |scope| {
    for i in 0..2 {
        scope.execute(move || unsafe { ptr::addr_of_mut!(X).write_volatile(i) });
    }
});

without the supressions file, the false positives seem to mask the real data race because the variable X is not included in the reported data races

it may be best to remove the scoped_threadpool dependency from the tests ...

japaric added a commit that referenced this pull request Apr 29, 2022
as it's easier to deal with TSAN false positives in the former API

as surfaced in PR 280 the current supression rules don't handle newer versions of the
scoped_threadpool crate

trying to update the supression rules related to scoped_threadpool in PR #282 revealed that the
supression rules are masking (hiding) real data races:
#282 (comment)

std::thread::scope requires less supression rules and does not mask real data races -- for instance,
the data race in the linked issue comment is not masked when using std::thread::scope

tradeoffs:
- pro: one less dev dependency
- pro: supressions file is simpler
- cons: std::thread::scope is only available on recent nightlies
japaric added a commit that referenced this pull request Apr 29, 2022
as it's easier to deal with TSAN false positives in the former API

as surfaced in PR 280 the current supression rules don't handle newer versions of the
scoped_threadpool crate

trying to update the supression rules related to scoped_threadpool in PR #282 revealed that the
supression rules are masking (hiding) real data races:
#282 (comment)

std::thread::scope requires less supression rules and does not mask real data races -- for instance,
the data race in the linked issue comment is not masked when using std::thread::scope

tradeoffs:
- pro: one less dev dependency
- pro: supressions file is simpler
- cons: std::thread::scope is only available on recent nightlies
@japaric
Copy link
Member Author

japaric commented Apr 29, 2022

closing in favor of PR #283

@japaric japaric closed this Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant