Skip to content

Commit

Permalink
arc: avoid panics in drops
Browse files Browse the repository at this point in the history
Panicing in drops can result in panic-in-panic events that make debugging
harder than necessary. Make an attempt to catch panics coming from e.g. the
execution being unavailable and just set a flag to cause the test to fail later
instead.
  • Loading branch information
Bryan Donlan committed Oct 26, 2020
1 parent cf0efdb commit f40cc37
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 6 deletions.
6 changes: 6 additions & 0 deletions src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,11 @@ impl Builder {

let f = f.clone();

let panic_in_drop_cell = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
let _enter = rt::panic::with_panic_in_drop_cell(panic_in_drop_cell.clone());

scheduler.run(&mut execution, move || {
let _enter = rt::panic::with_panic_in_drop_cell(panic_in_drop_cell);
f();

let lazy_statics = rt::execution(|execution| execution.lazy_statics.drop());
Expand All @@ -209,6 +213,8 @@ impl Builder {

execution.check_for_leaks();

rt::panic::check_panic_in_drop();

if let Some(next) = execution.step() {
execution = next;
} else {
Expand Down
53 changes: 53 additions & 0 deletions src/rt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,66 @@ pub(crate) const MAX_THREADS: usize = 4;
/// Maximum number of atomic store history to track per-cell.
pub(crate) const MAX_ATOMIC_HISTORY: usize = 7;

/// In some cases, we may need to suppress panics that occur in Drop handlers.
/// The thread-local state here provides a place to record that we did so, so
/// that we can force the test to fail, while allowing the drop processing to
/// proceed in the meantime.
pub(crate) mod panic {
use std::cell::RefCell;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;

thread_local! {
static PANIC_IN_DROP_CELL : RefCell<Option<Arc<AtomicBool>>> = RefCell::new(None);
}

pub(crate) fn paniced_in_drop() {
eprintln!("Suppressing panic occurring in drop handler");
PANIC_IN_DROP_CELL.with(|p| {
let borrow = p.borrow();
if let Some(atomic) = &*borrow {
atomic.store(true, Ordering::SeqCst);
}
});
}

pub(crate) fn get_panic_in_drop_cell() -> Option<Arc<AtomicBool>> {
PANIC_IN_DROP_CELL.with(|p| p.borrow().clone())
}

pub(crate) fn with_panic_in_drop_cell(cell: Arc<AtomicBool>) -> impl Drop {
struct ClearCell(Option<Arc<AtomicBool>>);
impl Drop for ClearCell {
fn drop(&mut self) {
PANIC_IN_DROP_CELL.with(|p| p.replace(self.0.take()));
}
}

PANIC_IN_DROP_CELL.with(|p| {
let mut p = p.borrow_mut();

let restore = ClearCell(p.take());
*p = Some(cell);
restore
})
}

pub(crate) fn check_panic_in_drop() {
if PANIC_IN_DROP_CELL.with(|p| p.borrow().as_ref().unwrap().load(Ordering::SeqCst)) {
panic!("Paniced in drop handler");
}
}
}

pub(crate) fn spawn<F>(f: F) -> crate::rt::thread::Id
where
F: FnOnce() + 'static,
{
let panic_in_drop = panic::get_panic_in_drop_cell().unwrap();
let id = execution(|execution| execution.new_thread());

Scheduler::spawn(Box::new(move || {
let _enter = panic::with_panic_in_drop_cell(panic_in_drop);
f();
thread_done();
}));
Expand Down
19 changes: 13 additions & 6 deletions src/sync/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,19 @@ impl<T> Clone for Arc<T> {
impl<T> Drop for Arc<T> {
#[track_caller]
fn drop(&mut self) {
if self.inner.obj.ref_dec(&trace!()) {
assert_eq!(
1,
std::sync::Arc::strong_count(&self.inner),
"something odd is going on"
);
let trace = trace!();
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
if self.inner.obj.ref_dec(&trace) {
assert_eq!(
1,
std::sync::Arc::strong_count(&self.inner),
"something odd is going on"
);
}
}));

if result.is_err() {
crate::rt::panic::paniced_in_drop();
}
}
}
Expand Down

0 comments on commit f40cc37

Please sign in to comment.