Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rc/Arc: don't leak the allocation if drop panics #132231

Merged
merged 3 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
add test for panicking drop in Box/Rc/Arc
  • Loading branch information
Lukas Markeffsky committed Oct 27, 2024
commit 8a9f40043fb6177a8a6d720103bac4e8889e0732
40 changes: 38 additions & 2 deletions library/alloc/tests/arc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::any::Any;
use std::cell::RefCell;
use std::cell::{Cell, RefCell};
use std::iter::TrustedLen;
use std::mem;
use std::sync::{Arc, Weak};
Expand Down Expand Up @@ -89,7 +89,7 @@ fn eq() {

// The test code below is identical to that in `rc.rs`.
// For better maintainability we therefore define this type alias.
type Rc<T> = Arc<T>;
type Rc<T, A = std::alloc::Global> = Arc<T, A>;

const SHARED_ITER_MAX: u16 = 100;

Expand Down Expand Up @@ -210,6 +210,42 @@ fn weak_may_dangle() {
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::sync::Weak`
}

/// Test that a panic from a destructor does not leak the allocation.
#[test]
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
fn panic_no_leak() {
use std::alloc::{AllocError, Allocator, Global, Layout};
use std::panic::{AssertUnwindSafe, catch_unwind};
use std::ptr::NonNull;

struct AllocCount(Cell<i32>);
unsafe impl Allocator for AllocCount {
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
self.0.set(self.0.get() + 1);
Global.allocate(layout)
}
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
self.0.set(self.0.get() - 1);
unsafe { Global.deallocate(ptr, layout) }
}
}

struct PanicOnDrop;
impl Drop for PanicOnDrop {
fn drop(&mut self) {
panic!("PanicOnDrop");
}
}

let alloc = AllocCount(Cell::new(0));
let rc = Rc::new_in(PanicOnDrop, &alloc);
assert_eq!(alloc.0.get(), 1);

let panic_message = catch_unwind(AssertUnwindSafe(|| drop(rc))).unwrap_err();
assert_eq!(*panic_message.downcast_ref::<&'static str>().unwrap(), "PanicOnDrop");
assert_eq!(alloc.0.get(), 0);
}

/// This is similar to the doc-test for `Arc::make_mut()`, but on an unsized type (slice).
#[test]
fn make_mut_unsized() {
Expand Down
38 changes: 38 additions & 0 deletions library/alloc/tests/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,44 @@ fn box_deref_lval() {
assert_eq!(x.get(), 1000);
}

/// Test that a panic from a destructor does not leak the allocation.
#[test]
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
fn panic_no_leak() {
use std::alloc::{AllocError, Allocator, Global, Layout};
use std::panic::{AssertUnwindSafe, catch_unwind};
use std::ptr::NonNull;

struct AllocCount(Cell<i32>);
unsafe impl Allocator for AllocCount {
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
self.0.set(self.0.get() + 1);
Global.allocate(layout)
}
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
self.0.set(self.0.get() - 1);
unsafe { Global.deallocate(ptr, layout) }
}
}

struct PanicOnDrop {
_data: u8,
}
impl Drop for PanicOnDrop {
fn drop(&mut self) {
panic!("PanicOnDrop");
}
}

let alloc = AllocCount(Cell::new(0));
let b = Box::new_in(PanicOnDrop { _data: 42 }, &alloc);
assert_eq!(alloc.0.get(), 1);

let panic_message = catch_unwind(AssertUnwindSafe(|| drop(b))).unwrap_err();
assert_eq!(*panic_message.downcast_ref::<&'static str>().unwrap(), "PanicOnDrop");
assert_eq!(alloc.0.get(), 0);
}

#[allow(unused)]
pub struct ConstAllocator;

Expand Down
38 changes: 37 additions & 1 deletion library/alloc/tests/rc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::any::Any;
use std::cell::RefCell;
use std::cell::{Cell, RefCell};
use std::iter::TrustedLen;
use std::mem;
use std::rc::{Rc, Weak};
Expand Down Expand Up @@ -206,6 +206,42 @@ fn weak_may_dangle() {
// borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::rc::Weak`
}

/// Test that a panic from a destructor does not leak the allocation.
#[test]
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
fn panic_no_leak() {
use std::alloc::{AllocError, Allocator, Global, Layout};
use std::panic::{AssertUnwindSafe, catch_unwind};
use std::ptr::NonNull;

struct AllocCount(Cell<i32>);
unsafe impl Allocator for AllocCount {
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
self.0.set(self.0.get() + 1);
Global.allocate(layout)
}
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
self.0.set(self.0.get() - 1);
unsafe { Global.deallocate(ptr, layout) }
}
}

struct PanicOnDrop;
impl Drop for PanicOnDrop {
fn drop(&mut self) {
panic!("PanicOnDrop");
}
}

let alloc = AllocCount(Cell::new(0));
let rc = Rc::new_in(PanicOnDrop, &alloc);
assert_eq!(alloc.0.get(), 1);

let panic_message = catch_unwind(AssertUnwindSafe(|| drop(rc))).unwrap_err();
assert_eq!(*panic_message.downcast_ref::<&'static str>().unwrap(), "PanicOnDrop");
assert_eq!(alloc.0.get(), 0);
}

#[allow(unused)]
mod pin_coerce_unsized {
use alloc::rc::{Rc, UniqueRc};
Expand Down