Calling {guard,defer}_on_unwind() while already thread::panicking()

[danielhenrymantilla]

Description

Best seen with a code snippet:

fn foo() {
    ::scopeguard::defer_on_unwind! {
        eprintln!("Aborting for soundness");
        ::std::process::abort();
    }
    unsafe {
        /* panic-unsafe stuff */
    }
}

// We also happen to call `foo()` from within a panic hook:
::std::panic::set_hook(Box::new(|_| {
    // …
    foo();
    // …
}));

// The following prints and aborts even when the `/* panic-unsafe stuff */`
// does *not* panic!!
::std::panic::catch_unwind(|| panic!()).ok();

When the panic hook calls foo(), the behavior of the resulting ScopeGuard is, effectively, that of a Strategy::Always-running guard.

• That is, even if the /* panic-unsafe stuff */ does not itself panic, the guard is nonetheless invoked when dropped: the eprintln!() message is printed to the screen (which is extraneous no matter the reason of the original panic), and the process aborts, even if the original panic wasn't going to result in an abort.

Indeed, the OnUnwind strategy defers to thread::panicking(), and thread::panicking() yields true in the context of a panic hook unconditionally.

Solutions

If we really wanted to fix this edge case, the Scopeguard::with_strategy() constructor would have to also store the bool state of thread::panicking() when constructed, and read it on drop to decide whether something ought to be run.

But I reckon that that would be quite a significant and churny change, and just for an edge case which users can actually work around by themselves (via a similar flag mechanism), by doing something along the following lines:

//! user.rs

// if we're already panicking, then any future panics will abort anyways.
let _abort_on_unwind = ::std::thread::panicking().not().then(|| {
    ::scopeguard::guard_on_unwind((), |()| {
        /* defer body */
    })
});

• (instead of doing defer_on_unwind! { /* defer body */ })

So I actually don't think it warrants any code changes from the library, just some extra documentation to warn of this edge case 🙂

[botahamec]

I just ran into this issue as well. In my case, I'm trying to figure out if a specific function panicked, so there aren't any changes to the function which could have helped me. I wrote my own function so that it doesn't run on panics created outside of a specific set of code.

use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};

pub fn handle_unwind<R, F: FnOnce() -> R, G: FnOnce()>(try_fn: F, catch: G) -> R {
	let try_fn = AssertUnwindSafe(try_fn);
	catch_unwind(try_fn).unwrap_or_else(|e| {
		catch();
		resume_unwind(e)
	})
}