with-authinfo-kludge: Do job-control to make interrupt characters work.
authorMark Wooding <mdw@distorted.org.uk>
Thu, 21 Sep 2017 07:51:50 +0000 (08:51 +0100)
committerMark Wooding <mdw@distorted.org.uk>
Thu, 21 Sep 2017 08:32:33 +0000 (09:32 +0100)
Terminal-based newsreaders use terminal interrupt characters for their
own purposes, i.e. slrn(1) arranges that C-g is the `VINTR' character
(i.e., it sends `SIGINT' to the foreground process group).  This is bad
unless we take evasive action: it makes `slrn' go ding and cancel some
UI action, but it also wipes out `with-authinfo-kludge' and its various
helper processes, and brings the parent shell back into the foreground
where it fights `slrn' over the terminal.

Avoid this by running the client in a separate process group, pushing it
into the foreground, and proxying signals and foregroundness back and
forth to keep the shell happy.  It's all a bit ugly and desperate, but
it works well in practice and doesn't involve ignoring signals
everywhere (which was my first attempt at solving this bug).

with-authinfo-kludge

index d2d62fe..7e575c4 100755 (executable)
@@ -33,7 +33,8 @@ use Fcntl qw(:mode);
 use File::stat;
 use Getopt::Long qw(:config gnu_compat bundling
                    require_order no_getopt_compat);
-use POSIX qw(:errno_h :fcntl_h :sys_wait_h);
+use POSIX qw(:errno_h :fcntl_h :sys_wait_h
+            setpgid tcgetpgrp tcsetpgrp);
 use Socket qw(/^[AP]F_/ /^SOCK_/ /^sockaddr_/
              getaddrinfo /^AI_/ /^EAI_/
              getnameinfo /^NI_/);
@@ -69,6 +70,8 @@ my $SESSDIR = undef;
 my %SERVMAP = ();
 my %CLIENT_NOIP = ();
 my %KIDMAP = ();
+my $MYPGID = getpgrp;
+my $TTYFD = undef;
 my $CLIENTKID = -1;
 
 ###--------------------------------------------------------------------------
@@ -757,9 +760,41 @@ sub wait_for_ssh () {
   else { inform "  all tunnels started ok"; }
 }
 
+## Collect a file descriptor for the controlling terminal.  It's totally not
+## a problem if this doesn't work: then we'll just live without the job
+## control stuff, which is fine because we only need it when terminals are
+## involved.
+$TTYFD = POSIX::open "/dev/tty", O_RDWR;
+
+sub maybe_foreground_client () {
+  ## If we're currently the foreground process group, then make the client be
+  ## the foreground instead.
+
+  if (defined $TTYFD && $MYPGID == tcgetpgrp $TTYFD) {
+    kill -CONT, $CLIENTKID
+      or sysfail "failed to wake client: $!";
+    tcsetpgrp $TTYFD, $CLIENTKID
+      or sysfail "failed to make client the foreground process group: $!";
+  }
+}
+
+sub maybe_stop_self () {
+  ## If the client is currently the foreground process group, then we should
+  ## background ourselves.
+
+  if (defined $TTYFD && $CLIENTKID == tcgetpgrp $TTYFD) {
+    kill -TSTP, $MYPGID
+      or sysfail "failed to suspend own process group: $!";
+  }
+}
+
+set_sighandler "CONT", sub {
+  maybe_foreground_client;
+};
+
 set_sighandler "CHLD", sub {
   KID: for (;;) {
-    defined (my $kid = waitpid -1, WNOHANG)
+    defined (my $kid = waitpid -1, WNOHANG | WUNTRACED)
       or sysfail "failed to reap child: $!";
     last KID if $kid <= 0;
     my $st = ${^CHILD_ERROR_NATIVE};
@@ -767,6 +802,9 @@ set_sighandler "CHLD", sub {
     if (WIFEXITED($st) && WEXITSTATUS($st) == 0) {
       $how = "exited successfully";
       $rc = 0;
+    } elsif (WIFSTOPPED($st)) {
+      maybe_stop_self if $kid == $CLIENTKID;
+      next KID;
     } elsif (WIFSIGNALED($st)) {
       my $sig = WTERMSIG($st);
       $how = "killed by signal $sig";
@@ -796,11 +834,13 @@ sub run_client (@) {
   defined (my $kid = myfork) or sysfail "failed to fork: $!";
   if (!$kid) {
     hack_noip_env \%CLIENT_NOIP, "$SESSDIR/noip-client";
+    setpgid $$, $$ or sysfail "failed to set kid process group: $!";
     my $prog = $args[0];
     exec @args or sysfail "failed to exec `$prog': $!";
   }
   $CLIENTKID = $kid;
   write_to_file "$SESSDIR/client.pid", "$kid\n";
+  maybe_foreground_client;
 }
 
 sub accept_loop () {