Subject: Re: bug#62093: [PATCH] Let processes read nothing from stdin in tramp

From: Aleksander Trofimowicz <[email-masked]>
Date: Mon, 04 Dec 2023 11:05:56 +0000

Hi Michael,

Michael Albinus <[email-masked]> writes:

> I've tried several approaches, none of them did satisfy me
> completely. So I have introduced a new user option
> tramp-pipe-stty-settings, which you can use for different stty
> settings. See appended patch. Let-binding this to "-icanon min 0 time 1"
> in magit-start-process has at least solved the problem as shown here in
> this bug report. I'm not very happy with this, but it is the best I
> could do now. Could you please check?
>
The patch works as expected. However given the default value of
tramp-pipe-stty-settings you opted for, as well as the contents of
additional commentary in the code provided by this patch, I have got an
irresistible feeling I failed to convey the consequences of maintaining
the status quo. So let me show two simple cases:

1) open a terminal window on your local host

1.1. confirm it works in the canonical mode

$ stty -a|grep icanon

1.2. write 4 "E" characters to a random file, type ^D once, then write 4
more "E"s, and finally type ^D twice:

$ cat > /tmp/testfile
EEEEEEEE

1.3. check the contents of the target file with a hex dump
utility. Assuming you don't use fancy locale settings, this should look
like the following:

$ xxd /tmp/testfile
00000000: 4545 4545 4545 4545                      EEEEEEEE

2) now the same workflow but in the non-canonical mode

2.1. disable the canonical mode:

$ stty -icanon; stty -a|grep icanon

2. write 4 "E" characters to a random file, type ^D once, then write 4
more "E"s, and finally type ^D twice, and observe that the special
characters are echoed back and a cat process refuses to terminate. Type
^C to actually terminate the process:

$ cat > /tmp/testfile2
EEEE^DEEEE^D^D^C

2.3 check the contents of the file

$ xxd /tmp/testfile2
00000000: 4545 4545 0445 4545 4504 04              EEEE.EEEE..

As you can see there are three additional bytes (carrying 0x04 values),
each one represents the VEOF character which in the canonical mode is
intercepted by the kernel TTY subsystem, and alters the way kernel
interacts with a user process. In the non-canonical mode it is simply
passed to userspace.

Indeed very few programs are prepared to handle those terminal special
characters, and if you use one that doesn't then at best it hangs. In
less favourable circumstances the process might silently corrupt data
along the way or produce other unpleasant results.

In the Magit case it is not Magit/Emacs itself that hangs, but a git
process upon a read syscall on pts/0 on a remote host:

$ ps axjf # filtered for brevity

   PPID     PID    PGID     SID TTY        TPGID STAT   UID   TIME COMMAND
      1     977     977     977 ?             -1 Ss       0   0:00 sshd: /usr/bin/sshd -D [listener] 0 of 10-100 startups
    977  251082  251082  251082 ?             -1 Ss       0   0:00  \_ sshd: guest [priv]
 251082  251087  251082  251082 ?             -1 S     1000   0:00  |   \_ sshd: guest@pts/0
 251087  251089  251089  251089 pts/0     251089 Ss+   1000   0:00  |       \_ git --no-pager --literal-pathspecs -c core.preloadindex=true -c log.showSignature=false -c color.ui=false -c color.diff=false apply --cached -p0 --ignore-space-change -

$ cat /proc/251089/status|head -n7
Name:	git
Umask:	0022
State:	S (sleeping)
Tgid:	251089
Ngid:	0
Pid:	251089
PPid:	251087
[...]

# cat /proc/251089/stack
[<0>] wait_woken+0x54/0x60
[<0>] n_tty_read+0x588/0x640
[<0>] tty_read+0x134/0x250
[<0>] vfs_read+0x1fe/0x350
[<0>] ksys_read+0x6f/0xf0
[<0>] do_syscall_64+0x5d/0x90
[<0>] entry_SYSCALL_64_after_hwframe+0x6e/0xd8

The git program in the apply mode expects to receive 0 upon read() in
order conclude its processing. If tramp-pipe-stty-settings is set to
'-icanon min 1 time 0', that is never going to happen, since Magit
resorts to use process-send-eof to designate the end of input data,
which in turn sends VEOF if it is detected that pty is in use (and it
is, since this is what Tramp does behind the scene). The VEOF character
looses its special meaning in the non-canonical mode hence a deadlock
ensues.

Given above I would argue the canonical mode should be set as the
default one.

>
> We have already ssh multiplexing. If you like to avoid pty's at all, you
> might try to use direct async processes:
>
> (info "(tramp)Improving performance of asynchronous remote processes")
>
In my case that's the optimal solution. Thank you!

--
Kind regards,
at