diff --git a/ext/crates/once/src/lib.rs b/ext/crates/once/src/lib.rs index 142954500..e783a4af2 100644 --- a/ext/crates/once/src/lib.rs +++ b/ext/crates/once/src/lib.rs @@ -169,16 +169,30 @@ impl Drop for OnceVec { 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);