std implementation of panic might not detect double panicking

[nagisa]

Implementation of std::panicking::on_panic may panic and not detect it is double-panicking, entering endless loop of panicking and overflowing stack.

[nagisa]

There’s two underlying reasons:

• Use of TLS, implementation of which panics;
• Use of write!, implementation of which panics.

[alexcrichton]

There will inevitably be some set of code which runs between the decision to panic and then the check to see if we're already panicking, so is there something actionable we can do here? Was a bug uncovered or are there specific mitigation tactics you have in mind?

[nagisa]

My primary concern is use of write! (there’s also a FIXME in relevant locations), this could be fixed by either catching panics in blocks where write! is used or implementing a panic-less version of write for on_panic use.

As far as TLS goes, the only case it can panic is when the value impls Drop. Since Cells of bools are primitive values, using a simple non-std implementation of thread locals should get rid of any path that may panic:

#[thread_local]
static PANICKING: Cell<bool> = Cell::new(false);

// and then in on_panic instead of PANICKING.with(||{}) simply use it as regular static which contains a Cell.

I’m mostly ignoring another thread_local LOCAL_STDERR here, because it can be retrieved later after we check for double panicking.

[alexcrichton]

I think this was addressed in #28631, but @nagisa can you confirm that your worries were taken care of?

[nagisa]

I don’t exactly remember cases in which I could trigger it, but it certainly looks like this issue has been resolved by the linked PR.