Fix imbalanced terminal protocols on SIGCHLD

We enable terminal protocols once at startup, and disable them before exit.
Additionally, we disable them while evaluating commands (see 8164855b7 (Disable
terminal protocols throughout evaluation, 2024-04-02))..

Thirdly, we re-enable protocols inside builtin read (where it's disabled
because we are evaluating something).  All of these three are scoped and
statically guaranteed to not leak into each others scopes.

There is another place where we enable protocols non-scoped: when we
receive a notification that a job is stopped. If this is ever hit, things
will be imbalanced and we'll fail to restore the right terminal state,
or (more likely) crash due the assertion in terminal_protocols_enable().
This code path used to be necessary when we disabled protocols only while
actually executing an external command but we changed that in 8164855b7,
so it should no longer be.  Remove it.

I haven't been able to find a test case, I'll try to do that later.

The main reason we changed the scope of protocols was focus reporting (#10408).
We have given up on that for now (outside tmux where I can't get it to work)
so we might want to reconsider and go back to the "optimized" approach of
enabling it for as long as possible. But this is simpler, easier to verify.
This commit is contained in:
Johannes Altmanninger 2024-04-21 15:46:53 +02:00
parent eb2ccc4ea9
commit 99bf3d0dbb
2 changed files with 1 additions and 5 deletions

View file

@ -431,7 +431,7 @@ pub fn update_wait_on_sequence_key_ms(vars: &EnvStack) {
pub static TERMINAL_PROTOCOLS: MainThread<RefCell<Option<TerminalProtocols>>> =
MainThread::new(RefCell::new(None));
pub fn terminal_protocols_enable() {
fn terminal_protocols_enable() {
assert!(TERMINAL_PROTOCOLS.get().borrow().is_none());
TERMINAL_PROTOCOLS
.get()

View file

@ -11,7 +11,6 @@ use crate::env::Statuses;
use crate::event::{self, Event};
use crate::flog::{FLOG, FLOGF};
use crate::global_safety::RelaxedAtomicBool;
use crate::input_common::terminal_protocols_enable;
use crate::io::IoChain;
use crate::job_group::{JobGroup, MaybeJobId};
use crate::parse_tree::ParsedSourceRef;
@ -1413,9 +1412,6 @@ fn process_mark_finished_children(parser: &Parser, block_ok: bool) {
let status = ProcStatus::from_waitpid(statusv);
handle_child_status(j, proc, &status);
if status.stopped() {
if is_interactive_session() {
terminal_protocols_enable();
}
j.group().set_is_foreground(false);
}
if status.continued() {