Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ext/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,5 @@ examples/benchmarks/%-concurrent: dummy
fi

miri:
cargo miri test -p once
MIRIFLAGS="-Zmiri-ignore-leaks" cargo miri test -p once
MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-disable-isolation" cargo miri test -p once --features concurrent
43 changes: 34 additions & 9 deletions ext/crates/once/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,16 +169,30 @@ impl<T> Drop for OnceVec<T> {
fn drop(&mut self) {
let len = self.len();

unsafe {
// The lock may be poisoned. Access is always safe because we have mutable reference,
// but if we can acquire the lock we want to drop the elements inside. If the lock is
// poisoned, then we are probably panicking so we don't care about memory leakage.
if let Ok(ooo) = self.ooo.lock() {
let ooo_iter = ooo.0.iter();
for entry in ooo_iter {
std::ptr::drop_in_place(self.entry_ptr(*entry));
}
if std::thread::panicking() {
// If we are panicking, the unsafe deallocation block at the end of this method might
// contain UB due to double-free. I'm (@JoeyBF) not sure why exactly this happens, but
// I'm definitely encountering this UB in some other code I'm writing. It's not the end
// of the world to cause UB when panicking, since we're already in an unrecoverable
// state, but having any kind of UB in the code makes me uneasy.
//
// There's probably more elegant solutions than just leaking all the memory, but that
// does solve the issue. A proper fix would need a solid understanding of the guarantees
// that are upheld while panicking. Again, since we're already panicking, it doesn't
// matter much.
return;
}

// The lock may be poisoned. Access is always safe because we have mutable reference, but if
// we can acquire the lock we want to drop the elements inside.
if let Ok(ooo) = self.ooo.lock() {
let ooo_iter = ooo.0.iter();
for entry in ooo_iter {
unsafe { std::ptr::drop_in_place(self.entry_ptr(*entry)) };
}
}

unsafe {
for idx in 0..MAX_OUTER_LENGTH {
// We have mutable reference so we can do whatever we want
let page = &mut *self.data.as_ptr().add(idx);
Expand Down Expand Up @@ -904,4 +918,15 @@ mod tests {
v.push_ooo(6, 7);
drop(v);
}

// This is just so that MIRI can check for UB when we panic
#[test]
#[should_panic]
fn test_drop_panic() {
let v: OnceVec<u32> = OnceVec::new();
v.push(4);
v.push(3);
v.push_ooo(6, 7);
panic!();
}
}