Skip to content

Commit

Permalink
Fix UB while panicking
Browse files Browse the repository at this point in the history
  • Loading branch information
JoeyBF committed Aug 14, 2024
1 parent b4d98c6 commit a774a4f
Showing 1 changed file with 23 additions and 9 deletions.
32 changes: 23 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

0 comments on commit a774a4f

Please sign in to comment.