Skip to content

Commit

Permalink
Rc/Arc: don't leak the allocation if drop panics
Browse files Browse the repository at this point in the history
  • Loading branch information
Lukas Markeffsky committed Oct 27, 2024
1 parent 8a9f400 commit 4a71a59
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 18 deletions.
19 changes: 8 additions & 11 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2256,17 +2256,14 @@ unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Rc<T, A> {
unsafe {
self.inner().dec_strong();
if self.inner().strong() == 0 {
// destroy the contained object
ptr::drop_in_place(Self::get_mut_unchecked(self));

// 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()));
}
// 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 };

// 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);
}
}
}
Expand Down
16 changes: 9 additions & 7 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1872,15 +1872,17 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
// 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
Expand Down

0 comments on commit 4a71a59

Please sign in to comment.