Hey I found this using my UB static analyzer while scanning top 5000 downloaded crates:
Ref publicly contains a shared reference (src/symbor/reference.rs:12-20) and publicly exposes it as &T through Deref (src/symbor/reference.rs:42-46). Requiring only T: Send is insufficient: sending a Ref<Cell<i32>> to another thread gives both threads non-atomic access to the same Cell, even though Cell<i32>: Send but !Sync.
Sample:
use std::cell::Cell;
use dlopen2::symbor::Ref;
let cell = Cell::new(0_i32);
let r = Ref::new(&cell);
std::thread::scope(|scope| {
let sender = scope.spawn(move || {
for _ in 0..100_000 { r.set(r.get() + 1); }
});
for _ in 0..100_000 { cell.set(cell.get() + 1); }
sender.join().unwrap();
});Miri result:
error: Undefined Behavior: Data race detected between (1) non-atomic write on thread `main` and (2) non-atomic read on thread `unnamed-1` at alloc160
--> /home/kevin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cell.rs:558:18
|
558 | unsafe { *self.value.get() }
| ^^^^^^^^^^^^^^^^^ (2) just happened here
|
help: and (1) occurred earlier here
--> src/main.rs:15:13
|
15 | cell.set(cell.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: this is on thread `unnamed-1`
= note: stack backtrace:
0: std::cell::Cell::<i32>::get
at /home/kevin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cell.rs:558:18: 558:35
1: main::{closure#0}::{closure#0}
at src/main.rs:11:23: 11:30
note: the last function in that backtrace got called indirectly due to this code
--> src/main.rs:9:22
|
9 | let sender = scope.spawn(move || {
| ______________________^
10 | | for _ in 0..100_000 {
11 | | r.set(r.get() + 1);
12 | | }
13 | | });
| |__________^
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
Fix requires T: Sync (not merely T: Send) for the Send implementation, e.g. unsafe impl<'lib, T: Sync> Send for Ref<'lib, T> {}, matching the existing Sync bound.
Hey I found this using my UB static analyzer while scanning top 5000 downloaded crates:
Refpublicly contains a shared reference (src/symbor/reference.rs:12-20) and publicly exposes it as&TthroughDeref(src/symbor/reference.rs:42-46). Requiring onlyT: Sendis insufficient: sending aRef<Cell<i32>>to another thread gives both threads non-atomic access to the sameCell, even thoughCell<i32>: Sendbut!Sync.Sample:
Miri result:
Fix requires
T: Sync(not merelyT: Send) for theSendimplementation, e.g.unsafe impl<'lib, T: Sync> Send for Ref<'lib, T> {}, matching the existingSyncbound.