ThreadSanitizer doesn't like RwLock? #257
Comments
|
oh fwiw we're running |
|
Also: if you want to test parking_lot with tsan, you just need to build/run your tests with:
Sadly this is limited to Rust nightly, and only on x64(?) linux. I don't have a linux machine setup right now so I can't try to reproduce this locally to create an STR. |
|
That's strange, we don't use fences in the rwlock code. |
|
Does this only happen in firefox or can you reproduce it with the testsuite? |
|
🎵 This hole, it was made for me 🎶 |
|
Happy to help you debug any tsan setup issues. (forgot to mention you also need to have the rust-src component installed via rustup) |
|
It was pretty painless! This reproduces with the test suite @Amanieu, TSan generates 21k lines of tracebacks about all the data races it asserts are present, to a point I am going to take a few more whacks at it to see if this can be thinned down to particular tests. |
|
OK, it seems like most of TSan's complaints are about condvar and its accusations of RwLock's sins against threads and sanity are confined mostly to test_rwlock_downgrade. RwLock traceback Finished test [unoptimized + debuginfo] target(s) in 0.04s
Running target/x86_64-unknown-linux-gnu/debug/deps/parking_lot-84f85dbe66928cc9
running 22 tests
test rwlock::tests::smoke ... ok
test rwlock::tests::test_clone ... ok
test rwlock::tests::test_get_mut ... ok
thread '' panicked at 'explicit panic', src/rwlock.rs:343:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test rwlock::tests::test_into_inner ... ok
thread '' panicked at 'explicit panic', src/rwlock.rs:214:13
test rwlock::tests::test_into_inner_drop ... ok
thread '' panicked at 'explicit panic', src/rwlock.rs:227:13
test rwlock::tests::test_issue_203 ... ok
thread '' panicked at 'explicit panic', src/rwlock.rs:188:13
test rwlock::tests::test_rw_arc_access_in_unwind ... ok
thread '' panicked at 'explicit panic', src/rwlock.rs:201:13
test rwlock::tests::frob ... ok
test rwlock::tests::test_rw_arc_no_poison_rr ... ok
test rwlock::tests::test_rw_arc ... ok
test rwlock::tests::test_rw_arc_no_poison_rw ... ok
==================
WARNING: ThreadSanitizer: data race (pid=8322)
Write of size 4 at 0x7b0800007fb8 by thread T18:
#0 parking_lot::rwlock::tests::test_rwlock_downgrade::{{closure}} /home/jubilee/projects/parking_lot/src/rwlock.rs:525 (parking_lot-84f85dbe66928cc9+0xa8067)
#1 std::sys_common::backtrace::__rust_begin_short_backtrace /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125 (parking_lot-84f85dbe66928cc9+0x12cc37)
#2 std::thread::Builder::spawn_unchecked::{{closure}}::{{closure}} /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:470 (parking_lot-84f85dbe66928cc9+0x15ca77)
#3 as core::ops::function::FnOnce<()>>::call_once /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:308 (parking_lot-84f85dbe66928cc9+0x180af7)
#4 std::panicking::try::do_call /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:381 (parking_lot-84f85dbe66928cc9+0x134c16)
#5 __rust_try 4ojbw4fi6t12cnc1:? (parking_lot-84f85dbe66928cc9+0x13814b)
#6 std::panicking::try /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:345 (parking_lot-84f85dbe66928cc9+0x132d0c)
#7 std::panic::catch_unwind /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:382 (parking_lot-84f85dbe66928cc9+0x181147)
#8 std::thread::Builder::spawn_unchecked::{{closure}} /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:469 (parking_lot-84f85dbe66928cc9+0x15826a)
#9 core::ops::function::FnOnce::call_once{{vtable-shim}} /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227 (parking_lot-84f85dbe66928cc9+0x118d67)
#10 as core::ops::function::FnOnce>::call_once /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1042 (parking_lot-84f85dbe66928cc9+0x4ed37b)
#11 as core::ops::function::FnOnce>::call_once /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1042 (parking_lot-84f85dbe66928cc9+0x4ed476)
#12 std::sys::unix::thread::Thread::new::thread_start /home/jubilee/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/unix/thread.rs:89 (parking_lot-84f85dbe66928cc9+0x38ce33)
|
|
All of TSan's other complaints about condvar are not about the simple tests but are about the webkit_queue_test suite specifically, on all of the tests that are more complicated than the "sanity check" tests. Logged here: https://gist.github.com/workingjubilee/100ec233e963c0d993748e421aa4b53d |
|
Awesome, thanks a ton! I've definitely seen issues with condvars before -- e.g. google/sanitizers#1259. It's possible tsan needs to be "taught" about the condvar API you're using. I'll look into it. |
|
@Amanieu is the Condvar impl in this crate based on any particular C++ impl in production today? Inspection of that code might tell us if there are accomodations that might be necessary for ThreadSanitizer. (It seems like Blink ripped out or at least renamed ParkingLot..?) |
|
It's based on the webkit |
|
It's possible wtf/Condition.h has been tuned to be tsan compatible. Is it similar to your implementation? |
|
Replacing all the memory orderings in |
|
For the queue tests I think this is because |
running the benchmarks with threadsanitizer fails with the parking_lot RwLock. It seems there was a similar issue before, so I will see if the parking_lot team is responsive here, since the parking_lot lock shows much better performance. Amanieu/parking_lot#257
I'm improving Firefox's tsan coverage, and it seems to be unable to understand parking_lot's RwLock:
It's a known issue that ThreadSanitizer gets confused by fences, and prefers atomic reads/writes. I haven't delved into the source for RwLock, but if it has this issue, and can be tweaked to work the same without using fences, that would be a big win for checking that code is concurrency-correct.
Archived tsan backtrace
The text was updated successfully, but these errors were encountered: