tty screwed up on ctrl-c

Wayne Davison wayned at samba.org
Fri Dec 19 06:39:06 EST 2003


On Wed, Dec 17, 2003 at 04:26:53PM -0600, John Van Essen wrote:
> But ssh has to have *some* opportunity to restore the settings,
> doesn't it?

Not necessarily.  One view is that ssh should be catching all fatal
signals during the prompting so that it can be sure to restore the tty
and then die.  However, looking at what we can do to avoid this glitch,
I might argue that we should either avoid sending SIGUSR1 to ssh at all
or we should make sure that SIGUSR1 is ignored before we run ssh.  The
only potential issue I can see is that our exit code might have been
depending on the fact that ssh dies on SIGUSR1 if it is generated during
the middle of a transfer.

Your suggestion to put the msleep() call into the sig_int() routine got
me to thinking about signal-handler safety.  I believe that the msleep()
routine should be OK, but I notice that several of our handlers call
exit_cleanup(), and I'm not sure that's a safe thing for them to do
these days (that routine is complicated enough that I can't be sure that
every possible cleanup path avoids calling malloc(), for instance).  I
guess we should visit this issue during the next release cycle.

In the long run, I think a change like this would be a better fix than
the msleep() kluge:

--- pipe.c	15 Dec 2003 18:45:07 -0000	1.5
+++ pipe.c	18 Dec 2003 19:07:45 -0000
@@ -75,6 +75,7 @@
 		set_blocking(STDIN_FILENO);
 		if (blocking_io > 0)
 			set_blocking(STDOUT_FILENO);
+		signal(SIGUSR1, SIG_IGN);
 		execvp(command[0], command);
 		rprintf(FERROR, "Failed to exec %s : %s\n",
 			command[0], strerror(errno));

but I can't be sure that won't cause problems, so it will have to wait
for the next release (note that this new signal() call only affects the
running of the remote shell).

In the meantime, perhaps we should just go with this for now:

--- rsync.c	6 Dec 2003 21:07:27 -0000	1.124
+++ rsync.c	18 Dec 2003 19:35:58 -0000
@@ -237,6 +237,15 @@
 
 void sig_int(void)
 {
+	/* KLUGE: if the user hits Ctrl-C while ssh is prompting
+	 * for a password, then our cleanup's sending of a SIGUSR1
+	 * signal to all our children may kill ssh before it has a
+	 * chance to restore the tty settings (i.e. turn echo back
+	 * on).  By sleeping for a short time, ssh gets a bigger
+	 * chance to do the right thing.  If child processes are
+	 * not ssh waiting for a password, then this tiny delay
+	 * shouldn't hurt anything. */
+	msleep(400);
 	exit_cleanup(RERR_SIGNAL);
 }
 

I think that should be safe enough to include in 2.6.0.

..wayne..



More information about the rsync mailing list