Catch panics in critical section of thread::scope

[Enkelmann]

The problem

The current implementation of the scope function for generating scoped threads contains a critical section: If an unwinding panic occurs after execution of the user-provided closure but before all scoped threads are successfully joined then a scoped thread may escape the scope. So we have to ensure that no unwinding panics can occur in this critical section.

Can an unwinding panic occur in the critical section?

Maybe? My investigation into this turned up some obvious candidates for panics, but it could be that they all are aborting panics:

• The call to wg.wait() may panic on an integer overflow when internally calling Arc::clone(). Luckily, the panic is implemented as an aborting panic.
• The call to scope.handles.lock().unwrap() may panic if the lock is poisoned. The only way I found to (theoretically) poison the lock is to cause an out-of-memory error. But we are lucky again, since out-of-memory errors themselves already cause aborting panics.
• The call to .collect() at the end of the critical section may panic when trying to allocate a Vec<_>. To be honest I am not sure whether this can only cause aborting out-of-memory panics or whether there is some corner case where this would cause an unwinding panic. My concern here is that even if we are sure that this only causes aborting panics right now, can we be sure that this is also true for future versions of Rust?

The proposed solution

Just wrap the critical section in a catch_unwind(..) block that promotes unwinding panics to aborting panics until all scoped threads are successfully joined.

[taiki-e]

Hmm, if I understand correctly, there is no way actually to cause this at this time. (That said, we can do this with zero cost using RAII guards that abort on drop.)

[Enkelmann]

Hmm, if I understand correctly, there is no way actually to cause this at this time.

At least I hope so. But there is already an accepted RFC for Rust to change the behaviour of out-of-memory panics to unwinding panics on user request. So when this is implemented (it may already be on nightly, I haven't checked) and hits stable, the current implementation will be unsound.

(That said, we can do this with zero cost using RAII guards that abort on drop.)

Ohh, good idea! I should change the code accordingly. Before I start reinventing the wheel: Do you know whether this particular guard is already implemented somewhere?

[taiki-e]

But there is already an accepted RFC for Rust to change the behaviour of out-of-memory panics to unwinding panics on user request. So when this is implemented (it may already be on nightly, I haven't checked) and hits stable, the current implementation will be unsound.

Oh, you are right. The linked RFC is not implemented yet (https://github.com/rust-lang/rust/issues/43596), but I found that there is an unstable feature that does something similar: https://github.com/rust-lang/rust/issues/51245

Do you know whether this particular guard is already implemented somewhere?

I don't remember if it's already in the crossbeam, but it's so small, so I don't mind duplication.

struct Bomb;

impl Drop for Bomb {
    fn drop(&mut self) {
        std::process::abort();
    }
}

let bomb = Bomb;

// processes that may panic...

mem::forget(bomb);