unison icon indicating copy to clipboard operation
unison copied to clipboard

Interrupting the text UI terminates the child process for `ssh` too early

Open tleedjarv opened this issue 2 years ago • 3 comments

When interrupting the text UI with Ctrl-C (or any ^C/break in terminal input) then the generated SIGINT will terminate ssh before unison manages to do cleanup.

The issue is that unison will execute ssh as a child process. If unison is executed from a terminal then the ssh child process will be in the same process group and using the same terminal (the same session). A SIGINT generated by the terminal is sent to all processes in the same process group. The end result is that the ssh process dies before unison manages to do the cleanup with the server. You can witness this by pressing Ctrl-C (with unison in the foreground) and then receiving an error "Failed due to lost connection".

I'm not sure what the solution to this is. ~~Creating a new process group for the child process before exec'ing ssh is an option. Even if this works, I wonder how portable it is (it's POSIX, but still) and whether this will cause endless unforeseen side-effects.~~ Creating a new pty like is done for ssh by the GUI is another option and probably a better one. This could also work in Windows. Edit: Just remembered that executing ssh in a different process group wouldn't work because ssh may need input from the terminal.

Originally posted by @tleedjarv in https://github.com/bcpierce00/unison/issues/810#issuecomment-1292485699

tleedjarv avatar Oct 27 '22 06:10 tleedjarv

Comment by gdt:

I am not at all sure that pty is better than pgrp, if pgrp is POSIX-specified and ptys are not.

They're both in POSIX. I think ptys are widely supported, either by POSIX interface, pre-POSIX SysV interface or BSD interface. Unlike pgrp, also recent Windows has ptys.

The problem with pgrp-based solution is that if ssh has a different pgrp then it is considered a background process. Background processes don't receive the terminal-generated SIGINT (good!). They also can't read terminal input (would receive SIGTTIN or if blocking/ignoring that then EIO). ssh must do user interaction (interactive auth, host verification, ...) on the terminal, not stdin/stdout (which are forwarded to/from the remote end).

tleedjarv avatar Oct 27 '22 06:10 tleedjarv

As it often happens, the most obvious and simple solutions end up being the correct and best ones. I had initially discarded the idea of blocking or ignoring SIGINT before spawning the child process (signal masks and handlers are inherited by the child) because I assumed that ssh would override the masks and handlers anyway. It turns out that at least OpenSSH doesn't. So blocking and ignoring both work. While ssh does set a handler for SIGINT, it does not do so if the signal is already being ignored. The devil is in the details and there are some caveats, as always, but in general this approach seems to fix this issue on POSIX-y platforms.

tleedjarv avatar Oct 29 '22 15:10 tleedjarv

Other than making use of a pty, I don't think a solution for Windows is even possible. I'm going to close this ticket with a POSIX-only fix.

tleedjarv avatar Oct 29 '22 16:10 tleedjarv