refactored common code from XXX_EndProcessDetect in wxGTK[12] and wxMac/wxCocoa into wxHandleProcessTermination itself; also removed code dealing with negative pids as we don't use them any more

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@52696 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

src/gtk/utilsgtk.cpp

@@ -213,46 +213,18 @@ const gchar *wx_pango_version_check (int major, int minor, int micro)
 extern "C" {
 static
 void GTK_EndProcessDetector(gpointer data, gint source,
-                            GdkInputCondition WXUNUSED(condition) )
+                            GdkInputCondition WXUNUSED(condition))
 {
-   wxEndProcessData *proc_data = (wxEndProcessData *)data;
+    wxEndProcessData * const
+        proc_data = wx_static_cast(wxEndProcessData *, data);
 
-   // has the process really terminated? unfortunately GDK (or GLib) seem to
+    // child exited, end waiting
-   // generate G_IO_HUP notification even when it simply tries to read from a
+    close(source);
-   // closed fd and hasn't terminated at all
-   int pid = (proc_data->pid > 0) ? proc_data->pid : -(proc_data->pid);
-   int status = 0;
-   int rc = waitpid(pid, &status, WNOHANG);
 
-    if ( rc == 0 )
+    // don't call us again!
-    {
+    gdk_input_remove(proc_data->tag);
-        // This can only happen if the child application closes our dummy
-        // pipe that is used to monitor its lifetime; in that case, our
-        // best bet is to pretend the process did terminate, because
-        // otherwise wxExecute() would hang indefinitely
-        // (OnExceptionWaiting() won't be called again, the descriptor
-        // is closed now).
-        wxLogDebug("Child process (PID %i) still alive, even though notification was received that it terminated.", pid);
-    }
-    else if ( rc == -1 )
-    {
-        // As above, if waitpid() fails, the best we can do is to log the
-        // error and pretend the child terminated:
-        wxLogSysError(_("Failed to check child process' status"));
-    }
 
-   // set exit code to -1 if something bad happened
+    wxHandleProcessTermination(proc_data);
-   proc_data->exitcode = (rc > 0 && WIFEXITED(status))
-                         ? WEXITSTATUS(status)
-                         : -1;
-
-   // child exited, end waiting
-   close(source);
-
-   // don't call us again!
-   gdk_input_remove(proc_data->tag);
-
-   wxHandleProcessTermination(proc_data);
 }
 }
 

src/gtk1/utilsgtk.cpp

@@ -134,32 +134,16 @@ static
 void GTK_EndProcessDetector(gpointer data, gint source,
                             GdkInputCondition WXUNUSED(condition) )
 {
-   wxEndProcessData *proc_data = (wxEndProcessData *)data;
+    wxEndProcessData * const
+        proc_data = wx_static_cast(wxEndProcessData *, data);
 
-   // has the process really terminated? unfortunately GDK (or GLib) seem to
+    // child exited, end waiting
-   // generate G_IO_HUP notification even when it simply tries to read from a
+    close(source);
-   // closed fd and hasn't terminated at all
-   int pid = (proc_data->pid > 0) ? proc_data->pid : -(proc_data->pid);
-   int status = 0;
-   int rc = waitpid(pid, &status, WNOHANG);
 
-   if ( rc == 0 )
+    // don't call us again!
-   {
+    gdk_input_remove(proc_data->tag);
-       // no, it didn't exit yet, continue waiting
-       return;
-   }
 
-   // set exit code to -1 if something bad happened
+    wxHandleProcessTermination(proc_data);
-   proc_data->exitcode = rc != -1 && WIFEXITED(status) ? WEXITSTATUS(status)
-                                                      : -1;
-
-   // child exited, end waiting
-   close(source);
-
-   // don't call us again!
-   gdk_input_remove(proc_data->tag);
-
-   wxHandleProcessTermination(proc_data);
 }
 }
 

src/mac/corefoundation/utilsexc_cf.cpp

@@ -36,44 +36,8 @@ extern "C" void WXCF_EndProcessDetector(CFSocketRef s,
                                         void const *WXUNUSED(data),
                                         void *info)
 {
-    wxEndProcessData * const proc_data = static_cast<wxEndProcessData*>(info);
-
-/// This code could reasonably be shared between wxMac/wxCocoa and wxGTK ///
-    // PID is always positive on UNIX but wx uses the sign bit as a flag.
-    int pid = (proc_data->pid > 0) ? proc_data->pid : -proc_data->pid;
-    int status = 0;
-    int rc = waitpid(pid, &status, WNOHANG);
-    if(rc == 0)
-    {
-        // Keep waiting in case we got a spurious notification
-        // NOTE: In my limited testing, this doesn't happen.
-        return;
-    }
-
-    if(rc == -1)
-    {   // Error.. really shouldn't happen but try to gracefully handle it
-        wxLogLastError(_T("waitpid"));
-        proc_data->exitcode = -1;
-    }
-    else
-    {   // Process ended for some reason
-        wxASSERT_MSG(rc == pid, _T("unexpected waitpid() return value"));
-
-        if(WIFEXITED(status))
-            proc_data->exitcode = WEXITSTATUS(status);
-        else if(WIFSIGNALED(status))
-            // wxGTK doesn't do this but why not?
-            proc_data->exitcode = -WTERMSIG(status);
-        else
-        {   // Should NEVER happen according to waitpid docs
-            wxLogError(wxT("waitpid indicates process exited but not due to exiting or signalling"));
-            proc_data->exitcode = -1;
-        }
-    }
-/// The above code could reasonably be shared between wxMac/wxCocoa and wxGTK ///
-
     /*
-        Either waitpid failed or the process ended successfully.  Either way,
+        Either our pipe was closed or the process ended successfully.  Either way,
         we're done.  It's not if waitpid is going to magically succeed when
         we get fired again.  CFSocketInvalidate closes the fd for us and also
         invalidates the run loop source for us which should cause it to
@@ -87,7 +51,7 @@ extern "C" void WXCF_EndProcessDetector(CFSocketRef s,
     CFSocketInvalidate(s);
 
     // Now tell wx that the process has ended.
-    wxHandleProcessTermination(proc_data);
+    wxHandleProcessTermination(static_cast<wxEndProcessData *>(info));
 }
 
 /*!

src/unix/utilsunx.cpp

@@ -1234,47 +1234,11 @@ int wxAppTraits::AddProcessCallback(wxEndProcessData *data, int fd)
     {
         wxEndProcessFDIOHandler(wxEndProcessData *data, int fd)
             : m_data(data), m_fd(fd)
-        {}
+        {
+        }
 
         virtual void OnReadWaiting()
-            { wxFAIL_MSG("this isn't supposed to happen"); }
-        virtual void OnWriteWaiting()
-            { wxFAIL_MSG("this isn't supposed to happen"); }
-
-        virtual void OnExceptionWaiting()
         {
-            const int pid = m_data->pid > 0 ? m_data->pid : -(m_data->pid);
-            int status = 0;
-
-            // has the process really terminated?
-            int rc = waitpid(pid, &status, WNOHANG);
-            if ( rc == 0 )
-            {
-                // This can only happen if the child application closes our
-                // dummy pipe that is used to monitor its lifetime; in that
-                // case, our best bet is to pretend the process did terminate,
-                // because otherwise wxExecute() would hang indefinitely
-                // (OnExceptionWaiting() won't be called again, the descriptor
-                // is closed now).
-                wxLogDebug("Child process (PID %d) still alive, "
-                           "even though pipe was closed.", pid);
-            }
-            else if ( rc == -1 )
-            {
-                // As above, if waitpid() fails, the best we can do is to log the
-                // error and pretend the child terminated:
-                wxLogSysError(_("Failed to check child process' status"));
-            }
-
-            // set exit code to -1 if something bad happened
-            m_data->exitcode = rc > 0 && WIFEXITED(status) ? WEXITSTATUS(status)
-                                                           : -1;
-
-            wxLogTrace("exec",
-                       "Child process (PID %d) terminated with exit code %d",
-                       pid, m_data->exitcode);
-
-            // child exited, end waiting
             wxFDIODispatcher::Get()->UnregisterFD(m_fd);
             close(m_fd);
 
@@ -1283,6 +1247,9 @@ int wxAppTraits::AddProcessCallback(wxEndProcessData *data, int fd)
             delete this;
         }
 
+        virtual void OnWriteWaiting() { wxFAIL_MSG("unreachable"); }
+        virtual void OnExceptionWaiting() { wxFAIL_MSG("unreachable"); }
+
         wxEndProcessData * const m_data;
         const int m_fd;
     };
@@ -1291,7 +1258,7 @@ int wxAppTraits::AddProcessCallback(wxEndProcessData *data, int fd)
                              (
                                  fd,
                                  new wxEndProcessFDIOHandler(data, fd),
-                                 wxFDIO_EXCEPTION
+                                 wxFDIO_INPUT
                              );
     return fd; // unused, but return something unique for the tag
 }
@@ -1378,6 +1345,59 @@ private:
 
 #endif // wxUSE_STREAMS
 
+// helper function which calls waitpid() and analyzes the result
+namespace
+{
+
+int DoWaitForChild(int pid, int flags = 0)
+{
+    wxASSERT_MSG( pid > 0, "invalid PID" );
+
+    int status, rc;
+
+    // loop while we're getting EINTR
+    for ( ;; )
+    {
+        rc = waitpid(pid, &status, flags);
+
+        if ( rc != -1 || errno != EINTR )
+            break;
+    }
+
+    if ( rc == 0 )
+    {
+        // This can only happen if the child application closes our dummy pipe
+        // that is used to monitor its lifetime; in that case, our best bet is
+        // to pretend the process did terminate, because otherwise wxExecute()
+        // would hang indefinitely (OnReadWaiting() won't be called again, the
+        // descriptor is closed now).
+        wxLogDebug("Child process (PID %d) still alive but pipe closed so "
+                   "generating a close notification", pid);
+    }
+    else if ( rc == -1 )
+    {
+        wxLogLastError(wxString::Format("waitpid(%d)", pid));
+    }
+    else // child did terminate
+    {
+        wxASSERT_MSG( rc == pid, "unexpected waitpid() return value" );
+
+        if ( WIFEXITED(status) )
+            return WEXITSTATUS(status);
+        else if ( WIFSIGNALED(status) )
+            return -WTERMSIG(status);
+        else
+        {
+            wxLogError("Child process (PID %d) exited for unknown reason, "
+                       "status = %d", pid, status);
+        }
+    }
+
+    return -1;
+}
+
+} // anonymous namespace
+
 int wxAppTraits::WaitForChild(wxExecuteData& execData)
 {
     if ( !(execData.flags & wxEXEC_SYNC) )
@@ -1421,81 +1441,22 @@ int wxAppTraits::WaitForChild(wxExecuteData& execData)
     //else: no IO redirection, just block waiting for the child to exit
 #endif // wxUSE_STREAMS
 
-    int status = 0;
+    return DoWaitForChild(execData.pid);
-
-    int result = waitpid(execData.pid, &status, 0);
-#ifdef __DARWIN__
-    /*  DE: waitpid manpage states that waitpid can fail with EINTR
-        if the call is interrupted by a caught signal.  I suppose
-        that means that this ought to be a while loop.
-
-        The odd thing is that it seems to fail EVERY time.  It fails
-        with a quickly exiting process (e.g. echo), and fails with a
-        slowly exiting process (e.g. sleep 2) but clearly after
-        having waited for the child to exit. Maybe it's a bug in
-        my particular version.
-
-        It works, however, from the CFSocket callback without this
-        trick but in that case it's used only after CFSocket calls
-        the callback and with the WNOHANG flag which would seem to
-        preclude it from being interrupted or at least make it much
-        less likely since it would not then be waiting.
-
-        If Darwin's man page is to be believed then this is definitely
-        necessary.  It's just weird that I've never seen it before
-        and apparently no one else has either or you'd think they'd
-        have reported it by now.  Perhaps blocking the GUI while
-        waiting for a child process to exit is simply not that common.
-     */
-    if ( result == -1 && errno == EINTR )
-    {
-        result = waitpid(execData.pid, &status, 0);
-    }
-#endif // __DARWIN__
-
-    if ( result == -1 )
-    {
-        wxLogLastError("waitpid");
-    }
-    else // child terminated
-    {
-        wxASSERT_MSG( result == execData.pid,
-                      "unexpected waitpid() return value" );
-
-        if ( WIFEXITED(status) )
-        {
-            return WEXITSTATUS(status);
-        }
-        else // abnormal termination?
-        {
-            wxASSERT_MSG( WIFSIGNALED(status),
-                          "unexpected child wait status" );
-        }
-    }
-
-    wxLogSysError(_("Waiting for subprocess termination failed"));
-
-    return -1;
 }
 
-void wxHandleProcessTermination(wxEndProcessData *proc_data)
+void wxHandleProcessTermination(wxEndProcessData *data)
 {
+    data->exitcode = DoWaitForChild(data->pid, WNOHANG);
+
     // notify user about termination if required
-    if ( proc_data->process )
+    if ( data->process )
     {
-        proc_data->process->OnTerminate(proc_data->pid, proc_data->exitcode);
+        data->process->OnTerminate(data->pid, data->exitcode);
     }
 
-    // clean up
+    // this function is only called for asynchronously executing children, now
-    if ( proc_data->pid > 0 )
+    // that we have received the notification about their termination there is
-    {
+    // no need to keep it around any longer
-        // async execution
+    delete data;
-        delete proc_data;
-    }
-    else // sync execution
-    {
-        // let wxExecute() know that the process has terminated
-        proc_data->pid = 0;
-    }
 }