Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data race reported by thread sanitizer when using upgradable read guard #306

Closed
NobodyXu opened this issue Dec 13, 2021 · 9 comments
Closed

Comments

@NobodyXu
Copy link

Hi

I've been working on my own project NobodyXu/concurrent_arena and I used the thread sanitizer of the unstable rust.

It reported that my use upgradable reader to access the Vec protected by RwLock has data race, and I have no idea how to fix this.

Here is the link to the code Arena::try_reserve that the data race occured in.

And here is the log of thread sanitizer error in the CI.

This issue is also reported in NobodyXu/concurrent_arena#1.

@bjorn3
Copy link
Contributor

bjorn3 commented Dec 13, 2021

https://github.com/NobodyXu/concurrent_arena/blob/80d6ee09d101b5cd9d40d712775a776ba7edca5b/src/bucket.rs#L87 is wrong. There is no lock preventing two threads from setting the UnsafeCell contents at the same time.

Tip: when you get a data race or memory corruption check all unsafe code you wrote. Most of the time the issue is there and not in any of your dependencies.

@NobodyXu
Copy link
Author

Thanks for taking the time to read my code!

However, I don't think that is the case here, since BitMap used here is guaranteed not to return an already allocated entry.

BitMap internally uses a AtomicUsize array and utilizes compare_exchange_weak to guarantee that.
The unit test for BitMap pass without any problem, so I doubt that is the case.

@Amanieu
Copy link
Owner

Amanieu commented Dec 13, 2021

That compare_exchange uses Relaxed ordering which is probably too weak. Try changing it to SeqCst to see if this resolves the race.

@NobodyXu
Copy link
Author

The test still failed

@NobodyXu
Copy link
Author

NobodyXu commented Dec 15, 2021

Could it be that the thread sanitizer implementation in rust is buggy?

It seems that the implementation still has some bugs and sometimes the linking just fails.

@Amanieu
Copy link
Owner

Amanieu commented Dec 15, 2021

I tried running this locally (x86_64 linux) but couldn't reproduce the issue.

In the past there have been some issues with RwLock upgrade but this should have been fixed by c866ba8.

@NobodyXu
Copy link
Author

That's strange since the github workflow can reproduce this data race error reliably.

It is using cargo-1.59.0-nightly installed using rustup, using image ubuntu 20.04.3 LTS.

@NobodyXu
Copy link
Author

Here is some additional information on my local x86_64 linux:

cargo 1.58.0-nightly (40dc28175 2021-12-06)
release: 1.58.0
commit-hash: 40dc281755137ee804bc9b3b08e782773b726e44
commit-date: 2021-12-06
host: x86_64-unknown-linux-gnu
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1l)
os: Linux 2.7 [64-bit]

clang --version:

clang version 13.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm/13/bin

gcc --version:

gcc (Gentoo 11.2.1_p20211127 p0) 11.2.1 20211127
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

@NobodyXu
Copy link
Author

@Amanieu I think that this might be a false positive, since thread sanitizer might actually catch an UB in the allocator.

In the other data race I found in arc-swap, the error can only be reproduced on x86-64-unknown-linux-gnu but not x86-64 MacOS.

So I will close this issue for now since I have no solid proof that parking_lot has data race.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants