fix wxExecute thread shutdown and free wxExecuteData even if the associated process is still running (bug 1863908); fix memory leaks when the asynchronously launched processes are still running in the sample too

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

samples/exec/exec.cpp

@@ -81,14 +81,18 @@ public:
 
 // Define an array of process pointers used by MyFrame
 class MyPipedProcess;
-WX_DEFINE_ARRAY_PTR(MyPipedProcess *, MyProcessesArray);
+WX_DEFINE_ARRAY_PTR(MyPipedProcess *, MyPipedProcessesArray);
+
+class MyProcess;
+WX_DEFINE_ARRAY_PTR(MyProcess *, MyProcessesArray);
 
 // Define a new frame type: this is going to be our main frame
 class MyFrame : public wxFrame
 {
 public:
-    // ctor(s)
+    // ctor and dtor
     MyFrame(const wxString& title, const wxPoint& pos, const wxSize& size);
+    virtual ~MyFrame();
 
     // event handlers (these functions should _not_ be virtual)
     void OnQuit(wxCommandEvent& event);
@@ -122,6 +126,9 @@ public:
     void OnProcessTerminated(MyPipedProcess *process);
     wxListBox *GetLogListBox() const { return m_lbox; }
 
+    // for MyProcess
+    void OnAsyncTermination(MyProcess *process);
+
 private:
     void ShowOutput(const wxString& cmd,
                     const wxArrayString& output,
@@ -129,30 +136,11 @@ private:
 
     void DoAsyncExec(const wxString& cmd);
 
-    void AddAsyncProcess(MyPipedProcess *process)
+    void AddAsyncProcess(MyProcess *process) { m_allAsync.push_back(process); }
-    {
-        if ( m_running.IsEmpty() )
-        {
-            // we want to start getting the timer events to ensure that a
-            // steady stream of idle events comes in -- otherwise we
-            // wouldn't be able to poll the child process input
-            m_timerIdleWakeUp.Start(100);
-        }
-        //else: the timer is already running
 
-        m_running.Add(process);
+    void AddPipedProcess(MyPipedProcess *process);
-    }
+    void RemovePipedProcess(MyPipedProcess *process);
 
-    void RemoveAsyncProcess(MyPipedProcess *process)
-    {
-        m_running.Remove(process);
-
-        if ( m_running.IsEmpty() )
-        {
-            // we don't need to get idle events all the time any more
-            m_timerIdleWakeUp.Stop();
-        }
-    }
 
     // the PID of the last process we launched asynchronously
     long m_pidLast;
@@ -174,7 +162,11 @@ private:
 
     wxListBox *m_lbox;
 
-    MyProcessesArray m_running;
+    // array of running processes with redirected IO
+    MyPipedProcessesArray m_running;
+
+    // array of all asynchrously running processes
+    MyProcessesArray m_allAsync;
 
     // the idle event wake up timer
     wxTimer m_timerIdleWakeUp;
@@ -501,6 +493,17 @@ MyFrame::MyFrame(const wxString& title, const wxPoint& pos, const wxSize& size)
 #endif // wxUSE_STATUSBAR
 }
 
+MyFrame::~MyFrame()
+{
+    // any processes left until now must be deleted manually: normally this is
+    // done when the associated process terminates but it must be still running
+    // if this didn't happen until now
+    for ( size_t n = 0; n < m_allAsync.size(); n++ )
+    {
+        delete m_allAsync[n];
+    }
+}
+
 // ----------------------------------------------------------------------------
 // event handlers: file and help menu
 // ----------------------------------------------------------------------------
@@ -637,20 +640,23 @@ void MyFrame::OnKill(wxCommandEvent& WXUNUSED(event))
 
 void MyFrame::DoAsyncExec(const wxString& cmd)
 {
-    wxProcess *process = new MyProcess(this, cmd);
+    MyProcess * const process = new MyProcess(this, cmd);
     m_pidLast = wxExecute(cmd, wxEXEC_ASYNC, process);
     if ( !m_pidLast )
     {
-        wxLogError( _T("Execution of '%s' failed."), cmd.c_str() );
+        wxLogError(_T("Execution of '%s' failed."), cmd.c_str());
 
         delete process;
     }
     else
     {
-        wxLogStatus( _T("Process %ld (%s) launched."),
+        wxLogStatus(_T("Process %ld (%s) launched."), m_pidLast, cmd.c_str());
-            m_pidLast, cmd.c_str() );
 
         m_cmdLast = cmd;
+
+        // the parent frame keeps track of all async processes as it needs to
+        // free them if we exit before the child process terminates
+        AddAsyncProcess(process);
     }
 }
 
@@ -772,7 +778,7 @@ void MyFrame::OnExecWithRedirect(wxCommandEvent& WXUNUSED(event))
         }
         else
         {
-            AddAsyncProcess(process);
+            AddPipedProcess(process);
         }
     }
 
@@ -801,9 +807,9 @@ void MyFrame::OnExecWithPipe(wxCommandEvent& WXUNUSED(event))
     long pid = wxExecute(cmd, wxEXEC_ASYNC, process);
     if ( pid )
     {
-        wxLogStatus( _T("Process %ld (%s) launched."), pid, cmd.c_str() );
+        wxLogStatus(_T("Process %ld (%s) launched."), pid, cmd.c_str());
 
-        AddAsyncProcess(process);
+        AddPipedProcess(process);
     }
     else
     {
@@ -1019,9 +1025,41 @@ void MyFrame::OnTimer(wxTimerEvent& WXUNUSED(event))
 
 void MyFrame::OnProcessTerminated(MyPipedProcess *process)
 {
-    RemoveAsyncProcess(process);
+    RemovePipedProcess(process);
 }
 
+void MyFrame::OnAsyncTermination(MyProcess *process)
+{
+    m_allAsync.Remove(process);
+
+    delete process;
+}
+
+void MyFrame::AddPipedProcess(MyPipedProcess *process)
+{
+    if ( m_running.IsEmpty() )
+    {
+        // we want to start getting the timer events to ensure that a
+        // steady stream of idle events comes in -- otherwise we
+        // wouldn't be able to poll the child process input
+        m_timerIdleWakeUp.Start(100);
+    }
+    //else: the timer is already running
+
+    m_running.Add(process);
+    m_allAsync.Add(process);
+}
+
+void MyFrame::RemovePipedProcess(MyPipedProcess *process)
+{
+    m_running.Remove(process);
+
+    if ( m_running.IsEmpty() )
+    {
+        // we don't need to get idle events all the time any more
+        m_timerIdleWakeUp.Stop();
+    }
+}
 
 void MyFrame::ShowOutput(const wxString& cmd,
                          const wxArrayString& output,
@@ -1052,8 +1090,7 @@ void MyProcess::OnTerminate(int pid, int status)
     wxLogStatus(m_parent, _T("Process %u ('%s') terminated with exit code %d."),
                 pid, m_cmd.c_str(), status);
 
-    // we're not needed any more
+    m_parent->OnAsyncTermination(this);
-    delete this;
 }
 
 // ----------------------------------------------------------------------------

src/msw/utilsexc.cpp

@@ -38,6 +38,7 @@
 #include "wx/process.h"
 #include "wx/thread.h"
 #include "wx/apptrait.h"
+#include "wx/vector.h"
 
 
 #include "wx/msw/private.h"
@@ -101,6 +102,13 @@ wxCreateHiddenWindow(LPCTSTR *pclassname, LPCTSTR classname, WNDPROC wndproc);
 static const wxChar *wxMSWEXEC_WNDCLASSNAME = wxT("_wxExecute_Internal_Class");
 static const wxChar *gs_classForHiddenWindow = NULL;
 
+// event used to wake up threads waiting in wxExecuteThread
+static HANDLE gs_heventShutdown = NULL;
+
+// handles of all threads monitoring the execution of asynchronously running
+// processes
+static wxVector<HANDLE> gs_asyncThreads;
+
 // ----------------------------------------------------------------------------
 // private types
 // ----------------------------------------------------------------------------
@@ -131,6 +139,43 @@ public:
     virtual bool OnInit() { return true; }
     virtual void OnExit()
     {
+        if ( gs_heventShutdown )
+        {
+            // stop any threads waiting for the termination of asynchronously
+            // running processes
+            if ( !::SetEvent(gs_heventShutdown) )
+            {
+                wxLogDebug(_T("Failed to set shutdown event in wxExecuteModule"));
+            }
+
+            ::CloseHandle(gs_heventShutdown);
+            gs_heventShutdown = NULL;
+
+            // now wait until they terminate
+            if ( !gs_asyncThreads.empty() )
+            {
+                const size_t numThreads = gs_asyncThreads.size();
+
+                if ( ::WaitForMultipleObjects
+                       (
+                        numThreads,
+                        &gs_asyncThreads[0],
+                        TRUE,   // wait for all of them to become signalled
+                        3000    // long but finite value
+                       ) == WAIT_TIMEOUT )
+                {
+                    wxLogDebug(_T("Failed to stop all wxExecute monitor threads"));
+                }
+
+                for ( size_t n = 0; n < numThreads; n++ )
+                {
+                    ::CloseHandle(gs_asyncThreads[n]);
+                }
+
+                gs_asyncThreads.clear();
+            }
+        }
+
         if ( *gs_classForHiddenWindow )
         {
             if ( !::UnregisterClass(wxMSWEXEC_WNDCLASSNAME, wxGetInstance()) )
@@ -146,6 +191,8 @@ private:
     DECLARE_DYNAMIC_CLASS(wxExecuteModule)
 };
 
+IMPLEMENT_DYNAMIC_CLASS(wxExecuteModule, wxModule)
+
 #if wxUSE_STREAMS && !defined(__WXWINCE__)
 
 // ----------------------------------------------------------------------------
@@ -281,23 +328,49 @@ static DWORD __stdcall wxExecuteThread(void *arg)
 {
     wxExecuteData * const data = (wxExecuteData *)arg;
 
-    if ( ::WaitForSingleObject(data->hProcess, INFINITE) != WAIT_OBJECT_0 )
+    // create the shutdown event if we're the first thread starting to wait
+    if ( !gs_heventShutdown )
     {
-        wxLogDebug(_T("Waiting for the process termination failed!"));
+        // create a manual initially non-signalled event object
+        gs_heventShutdown = ::CreateEvent(NULL, TRUE, FALSE, NULL);
+        if ( !gs_heventShutdown )
+        {
+            wxLogDebug(_T("CreateEvent() in wxExecuteThread failed"));
+        }
     }
 
-    // get the exit code
+    HANDLE handles[2] = { data->hProcess, gs_heventShutdown };
-    if ( !::GetExitCodeProcess(data->hProcess, &data->dwExitCode) )
+    switch ( ::WaitForMultipleObjects(2, handles, FALSE, INFINITE) )
     {
-        wxLogLastError(wxT("GetExitCodeProcess"));
+        case WAIT_OBJECT_0:
+            // process terminated, get its exit code
+            if ( !::GetExitCodeProcess(data->hProcess, &data->dwExitCode) )
+            {
+                wxLogLastError(wxT("GetExitCodeProcess"));
+            }
+
+            wxASSERT_MSG( data->dwExitCode != STILL_ACTIVE,
+                          wxT("process should have terminated") );
+
+            // send a message indicating process termination to the window
+            ::SendMessage(data->hWnd, wxWM_PROC_TERMINATED, 0, (LPARAM)data);
+            break;
+
+        case WAIT_OBJECT_0 + 1:
+            // we're shutting down but the process is still running -- leave it
+            // run but clean up the associated data
+            if ( !data->state )
+            {
+                delete data;
+            }
+            //else: exiting while synchronously executing process is still
+            //      running? this shouldn't happen...
+            break;
+
+        default:
+            wxLogDebug(_T("Waiting for the process termination failed!"));
     }
 
-    wxASSERT_MSG( data->dwExitCode != STILL_ACTIVE,
-                  wxT("process should have terminated") );
-
-    // send a message indicating process termination to the window
-    ::SendMessage(data->hWnd, wxWM_PROC_TERMINATED, 0, (LPARAM)data);
-
     return 0;
 }
 
@@ -321,7 +394,7 @@ LRESULT APIENTRY _EXPORT wxExecuteWindowCbk(HWND hWnd, UINT message,
         {
             // we're executing synchronously, tell the waiting thread
             // that the process finished
-            data->state = 0;
+            data->state = false;
         }
         else
         {
@@ -850,7 +923,7 @@ long wxExecute(const wxString& cmd, int flags, wxProcess *handler)
         return pi.dwProcessId;
     }
 
-    ::CloseHandle(hThread);
+    gs_asyncThreads.push_back(hThread);
 
 #if wxUSE_IPC && !defined(__WXWINCE__)
     // second part of DDE hack: now establish the DDE conversation with the