Our panic handler attempts a blocking read from stdin and only exits
after the user presses Enter.
This is unconventional behavior and might cause surprise but there is a
significant upside: crashes become more visible for terminals that don't
already detect crashes (see ecdc9ce1d (Install a panic handler to avoid
dropping crash stacktraces, 2024-03-24)).
As reported in 4d0aa2b5d (Fix panic handler, 2024-08-28), the panic handler
failed to exit fish if the panic happens on background threads. It would
only exit the background thread (like autosuggestion/highlight/history-pager
performer) itself. The fix was to abort the whole process.
Aborting has the additional upside of generating a coredump.
However since abort() skips stack unwinding, 4d0aa2b5d makes us no longer
restore the terminal on panic. In particular, if the terminal supports kitty
progressive enhancements, keys like ctrl-p will no longer work in say,
a Bash parent shell. So it broke 121680147 (Use RAII for restoring term
modes, 2024-03-24).
Fix this while still aborting to create coredumps. This means we can't use
RAII (for better or worse). The bad part is that we have to deal with added
complexity; we need to make sure that we set the AT_EXIT handler only after
all its inputs (like TERMINAL_MODE_ON_STARTUP) are initialized to a safe
value, but also before any damage has been done to the terminal. I guess we
can add a bunch of assertions.
Unfortunately, if a background thread panics, I haven't yet figured out how
to tell the main thread to do the blocking read. So the trick of "Press
Enter to exit", which allows users to attach a debugger doesn't yet work for
panics in background threads. We can probably figure that out later. Maybe
use pthread_kill(3)? Of course we still create coredumps, so that's fine.
As a temporary workaround, let's sleep for a bit so the user can at least
see that there is a crash & stacktrace.
One ugly bit here is that unit tests run AT_EXIT twice but it should be
idempotent.
We no longer use RAII for enabling/disabling these, so a full object is
overkill. Additionally this object doesn't allow us to recover from the case
where we receive SIGTERM while inside terminal_protocols_{enable,disable}.
We can simply run disable another time since they're idempotent. Untested.
rustc 1.80 now complains about features not declared in Cargo.toml and cfg
keys/values not declared by build.rs to protect against typos or misuse (you
think you're using the right condition but you're not). See
rust-lang/cargo#10554 and rust-lang/rust#82450.
(We're not actually using TSAN under CI at this time, but I do want to re-enable
it at some point — especially if we get multithreaded execution going — using
the rust-native TSAN configuration.)
I'll be updating the `rsconf` crate and patching `build.rs` accordingly to also
handle the warnings about unknown cfg values, but tsan is a feature and not a
cfg and these can be dealt with in `Cargo.toml` directly.
As implied by the changelog.
Unfortunately it's not obvious how to access the RefCell value in spite
of a potential (albeit unlikely) present mutable borrow. We need to use a
different type to make it work in such cases, hopefully doing that in future.
In future we could even use panic=abort and use this style of cleanup for
panics (instead of RAII).
