Fish fails to recover terminal with several process runners

[CGamesPlay]

Fish seems to have some problems resetting the terminal when using a variety of process runners.

Steps to reproduce:

1. Clone this directory, install Node and Yarn, and run yarn install in the directory.
2. Run yarn dev, wait about 10 seconds, press ^C.

Alternative steps, to demonstrate that this isn't really about Yarn:

1. Clone as above.
2. Install robo and printf "dev:\n command: node_modules/.bin/remix dev\n" > robo.yml
3. Run robo dev, wait about 10 seconds, press ^C.

If it went correctly, the terminal is not properly reset and arrow keys produce escape sequences, like in this recording. Pressing enter after the prompt appears will properly restore the terminal.

I'm reporting this as a fish bug because fish generally does try to recover the terminal before printing the prompt, and it's not getting it right in this case. Also, since yarn is built with Node and robo is built with go, so this is a cross-language issue. Note that running remix dev directly doesn't trigger this issue, which is why I believe it's related to process runners rather than with remix/node. My suspicion is that the remix process is outputting something during process shutdown, and this is interfering with fish's reset. But why is fish starting its reset before the processes have finished shutting down?

Fish version: 3.4.1 • macOS 10.14.6 • Darwin Ryans-Macbook.local 18.7.0 Darwin Kernel Version 18.7.0: Tue Jun 22 19:37:08 PDT 2021; root:xnu-4903.278.70~1/RELEASE_X86_64 x86_64

[faho]

Please give us the output of stty -a before and after running one of these things.

One possibility is that these things start a subprocess, over which fish has no control, and that restores terminal modes after the main process has exited. AFAIK we can't wait on arbitrary subprocesses, so this would have to be fixed in these things.

Note that node at least has a bug open about it restoring tty modes when it has no business doing so: nodejs/node#41143 (yes, the title is the wrong way around). So if yarn ran a node subprocess and that hangs around after yarn itself exits that could manifest like this. (it might be possible for us to grab the terminal earlier, which might instead cause that subprocess to hang once it messes with the terminal).

[CGamesPlay]

The output of stty -a is identical before and after running the command. Asciinema. The output is:

speed 9600 baud; 50 rows; 120 columns;
lflags: icanon isig iexten echo echoe -echok echoke -echonl echoctl
	-echoprt -altwerase -noflsh -tostop -flusho pendin -nokerninfo
	-extproc
iflags: -istrip icrnl -inlcr -igncr -ixon -ixoff ixany imaxbel -iutf8
	-ignbrk brkint -inpck -ignpar -parmrk
oflags: opost onlcr -oxtabs -onocr -onlret
cflags: cread cs8 -parenb -parodd hupcl -clocal -cstopb -crtscts -dsrflow
	-dtrflow -mdmbuf
cchars: discard = ^O; dsusp = ^Y; eof = ^D; eol = <undef>;
	eol2 = <undef>; erase = ^?; intr = ^C; kill = ^U; lnext = ^V;
	min = 1; quit = ^\; reprint = ^R; start = ^Q; status = ^T;
	stop = ^S; susp = ^Z; time = 0; werase = ^W;

One thing that tripped me up is that reset causes the output to change. That is, in a new terminal, running stty -a > before.txt; reset; stty -a > after.txt; diff -u before.txt after.txt produces the following diff, which has nothing to do with the bug:

--- before.txt	2022-05-12 08:59:26.000000000 +0300
+++ after.txt	2022-05-12 08:59:27.000000000 +0300
@@ -2,8 +2,8 @@
 lflags: icanon isig iexten echo echoe echok echoke -echonl echoctl
 	-echoprt -altwerase -noflsh -tostop -flusho pendin -nokerninfo
 	-extproc
-iflags: -istrip icrnl -inlcr -igncr -ixon -ixoff ixany imaxbel iutf8
-	-ignbrk brkint -inpck -ignpar -parmrk
+iflags: -istrip icrnl -inlcr -igncr ixon -ixoff -ixany imaxbel iutf8
+	-ignbrk brkint -inpck ignpar -parmrk
 oflags: opost onlcr -oxtabs -onocr -onlret
 cflags: cread cs8 -parenb -parodd hupcl -clocal -cstopb -crtscts -dsrflow
 	-dtrflow -mdmbuf

[CGamesPlay]

So after looking into the node issue, it seems like that's the underlying culprit: if I run the node process with </dev/null 2>&1 | cat then I don't see the bug. This is a workaround, although it's not perfect because this generally means that color will be disabled, since the output is not a tty.

There are a few things that are still unclear to me:

• If node is actually restoring terminal modes, why does it cause a problem, given that none of the processes in question changed any terminal modes?
• Is this the fault of the process manager (yarn, robo) for not waiting around? I will say that I'm unable to reproduce the issue with make (printf 'all:\n\tnode_modules/.bin/remix dev\n' > Makefile; make)
• Why doesn't stty -a show any differences?

It may be that fish doesn't really have any control here. If that's the case, then feel free to close this issue 🙂

[faho]

If node is actually restoring terminal modes, why does it cause a problem, given that none of the processes in question changed any terminal modes?

The idea is that node changes the modes after we have restored ours. We get no notification that happened, so we can't re-restore them.

At least that's the only theory I can come up with.

Why doesn't stty -a show any differences?

That's my fault - stty shows the external modes, not the ones we use for fish internally. Those have echo and such enabled because that's the default - programs like cat won't be setting any modes, so they need the simple normal set. You might try stty -a -f /dev/ttyX in another terminal (where /dev/ttyX is what tty prints in the main terminal).

Question: Do things go back to normal after you've run one other command? E.g. you run that node thing, that breaks the modes, then you run cat and it's fixed?

[CGamesPlay]

You might try stty -a -f /dev/ttyX in another terminal (where /dev/ttyX is what tty prints in the main terminal).

OK, yeah, now we've got lots of differences:

--- before.txt	2022-05-12 10:00:00.000000000 +0300
+++ after.txt	2022-05-12 10:00:18.000000000 +0300
@@ -1,13 +1,13 @@
 speed 38400 baud; 50 rows; 120 columns;
-lflags: -icanon isig -iexten -echo echoe echok echoke -echonl echoctl
+lflags: icanon isig iexten echo echoe echok echoke -echonl echoctl
 	-echoprt -altwerase -noflsh -tostop -flusho -pendin -nokerninfo
 	-extproc
-iflags: -istrip -icrnl -inlcr -igncr ixon -ixoff ixany imaxbel iutf8
-	-ignbrk brkint -inpck -ignpar -parmrk
+iflags: -istrip icrnl -inlcr -igncr ixon -ixoff -ixany imaxbel iutf8
+	-ignbrk brkint -inpck ignpar -parmrk
 oflags: opost onlcr -oxtabs -onocr -onlret
 cflags: cread cs8 -parenb -parodd hupcl -clocal -cstopb -crtscts -dsrflow
 	-dtrflow -mdmbuf
 cchars: discard = ^O; dsusp = ^Y; eof = ^D; eol = <undef>;
 	eol2 = <undef>; erase = ^?; intr = ^C; kill = ^U; lnext = ^V;
 	min = 1; quit = ^\; reprint = ^R; start = ^Q; status = ^T;
-	stop = ^S; susp = <undef>; time = 0; werase = ^W;
+	stop = ^S; susp = ^Z; time = 0; werase = ^W;

Question: Do things go back to normal after you've run one other command? E.g. you run that node thing, that breaks the modes, then you run cat and it's fixed?

Yes, even pressing enter with no command fixes it (you can see this behavior in my this asciinema).

So really what's happening (I'm guessing) is:

1. Fish sets a variety of terminal modes before executing robo.
2. Robo executes node. Nodes saves the terminal settings.
3. ^C is pressed. Robo exits immediately without waiting for node to finish.
4. Fish, having finished waiting on the process it ran, takes back over the terminal, restores modes, and prints the prompt.
5. Node restores the saved terminal settings, then exits.

So the culprit here really is that these process managers (both yarn and robo) are abandoning their children (make doesn't do this, so it doesn't have the behavior).

Now, is it possible for fish to cover for these buggy process managers? I'm out of my depth here, but there's nothing with process groups/sessions that could be used for this?

[faho]

OK, yeah, now we've got lots of differences:

Ah, yeah, what mostly breaks things here is echo, which causes the terminal to print out stuff on its own, while fish wants tighter control over the output so it can do syntax highlighting and stuff.

Now, is it possible for fish to cover for these buggy process managers?

Not that I'm aware of, no. We already put these things in their own process group, which seems to not be enough to stop it.

(I have an issue here with my understanding of process groups, in that I assumed you can only change terminal modes if you are in the controlling pgroup, but that appears to not be the case - either fish wouldn't manage to restore them because it's not yet controlling, or the child wouldn't manage to restore it because it's not controlling anymore. I'd have to read up on this stuff some more)

[faho]

As far as I know there's nothing better we can do here, so I'm closing this.

If anyone manages to figure out that there's something we can do, we will of course consider it.