Open
Description
openedon Nov 1, 2023
In #115546 the memory ordering of {Arc,Weak}::{strong,weak}_count got changed to Relaxed, which can cause some hard to debug race bugs.
I tried this code:
use std::cell::UnsafeCell;
use std::sync::Arc;
use std::thread;
struct SendMe<T>(T);
unsafe impl<T> Send for SendMe<T> {}
fn main() {
let a = Arc::new(UnsafeCell::new(0i32));
let b = SendMe(a.clone());
thread::spawn(move || {
let b = b;
for _ in 0..100 {
unsafe { *b.0.get() += 1; }
}
});
while Arc::strong_count(&a) != 1 {}
// core::sync::atomic::fence(core::sync::atomic::Ordering::Acquire);
unsafe { *a.get() += 1; }
}I expected to see this happen: I expected the check strong_count == 0 to be strong enough to avoid a data race in this program. My intuition was that this check implies that no other thread can access the data concurrently.
Instead, this happened: With the fence commented out, the above program has undefined behavior. Miri outputs:
error: Undefined Behavior: Data race detected between (1) Write on thread `<unnamed>` and (2) Read on thread `main` at alloc826+0x10. (2) just happened here
--> src/main.rs:19:14
|
19 | unsafe { *a.get() += 1; }
| ^^^^^^^^^^^^^ Data race detected between (1) Write on thread `<unnamed>` and (2) Read on thread `main` at alloc826+0x10. (2) just happened here
|
help: and (1) occurred earlier here
--> src/main.rs:14:22
|
14 | unsafe { *b.0.get() += 1; }
| ^^^^^^^^^^^^^^^
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE (of the first span):
= note: inside `main` at src/main.rs:19:14: 19:27
I think we should either put the Acquire ordering back or add a warning to the docs that such code requires a manual fence.
Meta
1.74 = current beta
cc @SUPERCILEX author of #115546
@rustbot label T-libs A-atomic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Metadata
Assignees
Labels
Area: Atomics, barriers, and sync primitivesArea: Atomics, barriers, and sync primitivesCategory: Discussion or questions that doesn't represent real issues.Category: Discussion or questions that doesn't represent real issues.Relevant to the library team, which will review and decide on the PR/issue.Relevant to the library team, which will review and decide on the PR/issue.