From 4a71a592819b6fe87377dd5f857218ed37dbf195 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 27 Oct 2024 17:48:15 +0100 Subject: [PATCH] Rc/Arc: don't leak the allocation if drop panics --- library/alloc/src/rc.rs | 17 +++++++---------- library/alloc/src/sync.rs | 16 +++++++++------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 9fdd51ce331..52ec8237d83 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -2256,17 +2256,14 @@ fn drop(&mut self) { unsafe { self.inner().dec_strong(); if self.inner().strong() == 0 { - // destroy the contained object - ptr::drop_in_place(Self::get_mut_unchecked(self)); + // Reconstruct the "strong weak" pointer and drop it when this + // variable goes out of scope. This ensures that the memory is + // deallocated even if the destructor of `T` panics. + let _weak = Weak { ptr: self.ptr, alloc: &self.alloc }; - // remove the implicit "strong weak" pointer now that we've - // destroyed the contents. - self.inner().dec_weak(); - - if self.inner().weak() == 0 { - self.alloc - .deallocate(self.ptr.cast(), Layout::for_value_raw(self.ptr.as_ptr())); - } + // Destroy the contained object. + // We cannot use `get_mut_unchecked` here, because `self.alloc` is borrowed. + ptr::drop_in_place(&mut (*self.ptr.as_ptr()).value); } } } diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 15a1b0f2834..98a2fe24257 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1872,15 +1872,17 @@ fn inner(&self) -> &ArcInner { // Non-inlined part of `drop`. #[inline(never)] unsafe fn drop_slow(&mut self) { + // Drop the weak ref collectively held by all strong references when this + // variable goes out of scope. This ensures that the memory is deallocated + // even if the destructor of `T` panics. + // Take a reference to `self.alloc` instead of cloning because 1. it'll last long + // enough, and 2. you should be able to drop `Arc`s with unclonable allocators + let _weak = Weak { ptr: self.ptr, alloc: &self.alloc }; + // Destroy the data at this time, even though we must not free the box // allocation itself (there might still be weak pointers lying around). - unsafe { ptr::drop_in_place(Self::get_mut_unchecked(self)) }; - - // Drop the weak ref collectively held by all strong references - // Take a reference to `self.alloc` instead of cloning because 1. it'll - // last long enough, and 2. you should be able to drop `Arc`s with - // unclonable allocators - drop(Weak { ptr: self.ptr, alloc: &self.alloc }); + // We cannot use `get_mut_unchecked` here, because `self.alloc` is borrowed. + unsafe { ptr::drop_in_place(&mut (*self.ptr.as_ptr()).data) }; } /// Returns `true` if the two `Arc`s point to the same allocation in a vein similar to