From: Vadim Zeitlin Date: Sat, 22 Mar 2008 14:50:44 +0000 (+0000) Subject: refactored common code from XXX_EndProcessDetect in wxGTK[12] and wxMac/wxCocoa into... X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/b827d64707b39ea779f32e216c3261ef1d861503 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 --- diff --git a/src/gtk/utilsgtk.cpp b/src/gtk/utilsgtk.cpp index f557d87d97..d255eb50e0 100644 --- a/src/gtk/utilsgtk.cpp +++ b/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 - // generate G_IO_HUP notification even when it simply tries to read from a - // 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); + // child exited, end waiting + close(source); - 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 %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 - 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); + // don't call us again! + gdk_input_remove(proc_data->tag); - wxHandleProcessTermination(proc_data); + wxHandleProcessTermination(proc_data); } } diff --git a/src/gtk1/utilsgtk.cpp b/src/gtk1/utilsgtk.cpp index a0d3f1a54b..3cf71d33d2 100644 --- a/src/gtk1/utilsgtk.cpp +++ b/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 - // generate G_IO_HUP notification even when it simply tries to read from a - // 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); + // child exited, end waiting + close(source); - if ( rc == 0 ) - { - // no, it didn't exit yet, continue waiting - return; - } + // don't call us again! + gdk_input_remove(proc_data->tag); - // set exit code to -1 if something bad happened - 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); + wxHandleProcessTermination(proc_data); } } diff --git a/src/mac/corefoundation/utilsexc_cf.cpp b/src/mac/corefoundation/utilsexc_cf.cpp index 2ab2dbd059..2d95454fe9 100644 --- a/src/mac/corefoundation/utilsexc_cf.cpp +++ b/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(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(info)); } /*! diff --git a/src/unix/utilsunx.cpp b/src/unix/utilsunx.cpp index 305c194cf7..c87ccd2a6d 100644 --- a/src/unix/utilsunx.cpp +++ b/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; - - 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; + return DoWaitForChild(execData.pid); } -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 - if ( proc_data->pid > 0 ) - { - // async execution - delete proc_data; - } - else // sync execution - { - // let wxExecute() know that the process has terminated - proc_data->pid = 0; - } + // this function is only called for asynchronously executing children, now + // that we have received the notification about their termination there is + // no need to keep it around any longer + delete data; }