command substition causes fish to hang

[EvanTheB]

I am experiencing a bug with a command substitution:

> ./fish --no-config
> echo (cros_sdk -- echo 1)
1
> fish: Job 1, './fish --no-config' terminated

It looks like things work, but after the new prompt is drawn fish is unresponsive. The keyboard is ignored as well as ctrl-c etc. With some related reproductions of the bug fish closes immediately when the first keyboard character is typed.

The reproduction is related to cros_sdk doc source, which is a complex command that enters chroots and does other arcane things. I would like to find a more minimal reproduction if that helps, but could do with some guidance.

I bisected the issue to this commit, which seems very relevant: 2ca66cf

Attached a log from:

./fish --debug=event,exec\*,iothread,output-invalid,proc\*,reader\* --debug-output=/tmp/fish.log --no-config

fish.log

cros_sdk bug

[ridiculousfish]

Nice find, thank you for the repro steps. From what you described this sounds like fish getting a SIGTTIN signal, I'll see if I can repro it.

[ridiculousfish]

I was able to reproduce and confirmed that fish is receivingSIGTTIN, which means it tries to read from the terminal, but is signaled because it does not own the terminal. This suggests that cros_sdk is transferring away control of the tty (via tcsetpgrp) to some other process group, and not restoring it.

I tested with zsh and it appears to attempt to reclaim the terminal, even though it did not transfer it. This seems reasonable; probably fish can do the same.

[EvanTheB]

Thank you,

I confirmed, cros_sdk calls os.tcsetpgrp(fd, pgrp) but not if --no-ns-pid

https://source.chromium.org/chromiumos/chromiumos/codesearch/+/cf63316862706362f4c86849de540cad99518aba:chromite/lib/namespaces.py;l=234

echo (cros_sdk --no-ns-pid -- echo 1) Does not repro.

Do you have any document about how tcsetpgrp ought to be restored? I could fix this on the cros_sdk side in parallel.

[ridiculousfish]

This should be fixed in 5cf0778, please let me know if you still see it.

It would be good for cros_sdk to restore the foreground process group. A simple way is to fetch the current owner and restore that afterwards:

init_owner = os.tcgetpgrp(fd)
try:
    os.tcsetpgrp(fd, pgrp)
    do_stuff()
finally:
   os.tcsetpgrp(fd, init_owner)