[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kdesrc-build] modules/ksb: async: Improve signal handling.
From:       Michael Pyne <mpyne () kde ! org>
Date:       2014-03-24 4:29:46
Message-ID: E1WRwWA-0003e4-FJ () scm ! kde ! org
[Download RAW message or body]

Git commit b765b5d62d42fa6a33a84173694e179170b96dcd by Michael Pyne.
Committed on 24/03/2014 at 04:25.
Pushed by mpyne into branch 'master'.

async: Improve signal handling.

I was noticing that sometimes Ctrl-C at the TTY doing a kdesrc-build -p
left a monitor process hanging at 100% CPU, not to mention all the
kdesrc-build processes spamming goodbye messages.

It didn't help that I was doing waitpid on an updater PID when that
updater PID had to be 0 by definition.

Either way, I've verified that the forking going on here shouldn't start
a different process group for the shell session, so ^C is already going
to all of the relevant processes. We just need to make sure to catch the
signal and do the right thing (i.e. exit the child process).

M  +11   -16   modules/ksb/Application.pm
M  +14   -7    modules/ksb/Util.pm

http://commits.kde.org/kdesrc-build/b765b5d62d42fa6a33a84173694e179170b96dcd

diff --git a/modules/ksb/Application.pm b/modules/ksb/Application.pm
index 5e06ada..339ab64 100644
--- a/modules/ksb/Application.pm
+++ b/modules/ksb/Application.pm
@@ -29,7 +29,7 @@ use ksb::Version qw(scriptVersion);
 use List::Util qw(first min);
 use File::Basename; # basename, dirname
 use File::Glob ':glob';
-use POSIX qw(:sys_wait_h _exit);
+use POSIX qw(:sys_wait_h _exit :errno_h);
 use Getopt::Long qw(GetOptionsFromArray :config gnu_getopt nobundling pass_through);
 use IO::Handle;
 use IO::Select;
@@ -1787,34 +1787,24 @@ sub _handle_async_build
         my $updaterToMonitorIPC = ksb::IPC::Pipe->new();
         my $updaterPid = fork;
 
+        $SIG{INT} = sub { POSIX::_exit(EINTR); };
+
         if ($updaterPid) {
+            $0 = 'kdesrc-build-updater';
             $updaterToMonitorIPC->setSender();
             ksb::Debug::setIPC($updaterToMonitorIPC);
 
-            # Avoid calling close subroutines in more than one routine.
             POSIX::_exit (_handle_updates ($updaterToMonitorIPC, $ctx));
         }
         else {
+            $0 = 'kdesrc-build-monitor';
             $ipc->setSender();
             $updaterToMonitorIPC->setReceiver();
 
             $ipc->setLoggedModule('#monitor#'); # This /should/ never be used...
             ksb::Debug::setIPC($ipc);
 
-            $SIG{'INT'} = sub {
-                say "Received SIGQUIT, shutting down monitor.";
-                kill 'INT', $updaterPid;
-                waitpid ($updaterPid, 0);
-                POSIX::_exit ($result);
-            };
-
-            # Avoid calling close subroutines in more than one routine.
-            my $result = _handle_monitoring ($ipc, $updaterToMonitorIPC);
-
-            waitpid ($updaterPid, 0);
-            $result = 1 if $? != 0;
-
-            POSIX::_exit ($result);
+            POSIX::_exit (_handle_monitoring ($ipc, $updaterToMonitorIPC));
         }
     }
     else {
@@ -1945,6 +1935,11 @@ sub _handle_monitoring
         my ($readReadyRef, $writeReadyRef) =
             IO::Select->select($readSelector, $writeSelector, undef))
     {
+        if (!$readReadyRef && !$writeReadyRef) {
+            # Some kind of error occurred.
+            return 1;
+        }
+
         # Check for source updates first.
         if (@{$readReadyRef})
         {
diff --git a/modules/ksb/Util.pm b/modules/ksb/Util.pm
index affa670..ed13f9c 100644
--- a/modules/ksb/Util.pm
+++ b/modules/ksb/Util.pm
@@ -438,15 +438,17 @@ sub log_command
         # Let callback know there is no more output.
         &{$callbackRef}(undef) if defined $callbackRef;
 
-        close CHILD;
+        # This implicitly does a waitpid() as well
+        close CHILD or do {
+            if ($! == 0) {
+                _setErrorLogfile($module, "$filename.log");
+                return $?;
+            }
 
-        # If the module fails building, set an internal flag in the module
-        # options with the name of the log file containing the error message.
-        # TODO: ($? is set when closing CHILD pipe?)
-        my $result = $?;
-        _setErrorLogfile($module, "$filename.log") if $result;
+            return 1;
+        };
 
-        return $result;
+        return 0;
     }
     else
     {
@@ -470,6 +472,11 @@ sub log_command
         }
 
         $SIG{PIPE} = "IGNORE";
+        $SIG{INT} = sub {
+            close (STDOUT); # This should be a pipe
+            close (STDERR);
+            POSIX::_exit(EINTR);
+        };
 
         # Redirect STDIN to /dev/null so that the handle is open but fails when
         # being read from (to avoid waiting forever for e.g. a password prompt
[prev in list] [next in list] [prev in thread] [next in thread]