From 1a5c93ff19340ea993de211de10c3a72c4757956 Mon Sep 17 00:00:00 2001 From: Charlie Fenton Date: Tue, 18 Nov 2008 13:28:26 +0000 Subject: [PATCH] MGR: Async RPCs: use wxCondition with timeout to block main thread while waiting for Demand RPC to finish; set m_pTaskBarIcon and m_pMacSystemMenu to NULL when deleted. svn path=/trunk/boinc/; revision=16517 --- clientgui/AsyncRPC.cpp | 132 +++++++++++++++++++++++------------ clientgui/AsyncRPC.h | 9 ++- clientgui/BOINCBaseFrame.cpp | 9 +-- clientgui/BOINCGUIApp.cpp | 18 +++++ clientgui/BOINCGUIApp.h | 2 + clientgui/MainDocument.cpp | 25 +++++-- clientgui/MainDocument.h | 3 +- clientgui/mac/MacSysMenu.cpp | 7 +- 8 files changed, 142 insertions(+), 63 deletions(-) diff --git a/clientgui/AsyncRPC.cpp b/clientgui/AsyncRPC.cpp index 6467b74549..268c5642d2 100755 --- a/clientgui/AsyncRPC.cpp +++ b/clientgui/AsyncRPC.cpp @@ -30,6 +30,8 @@ // Delay in milliseconds before showing AsyncRPCDlg #define RPC_WAIT_DLG_DELAY 1500 +// How often to check for events when minimized and waiting for Demand RPC +#define DELAY_WHEN_MINIMIZED 500 // Delay in milliseconds to allow thread to exit before killing it #define RPC_KILL_DELAY 100 @@ -99,10 +101,17 @@ int AsyncRPC::RPC_Wait(RPC_SELECTOR which_rpc, void *arg1, void *arg2, } -RPCThread::RPCThread(CMainDocument *pDoc, wxMutex* pRPC_Thread_Mutex, wxCondition* pRPC_Thread_Condition) : wxThread() { +RPCThread::RPCThread(CMainDocument *pDoc, + wxMutex* pRPC_Thread_Mutex, + wxCondition* pRPC_Thread_Condition, + wxMutex* pRPC_Request_Mutex, + wxCondition* pRPC_Request_Condition) + : wxThread() { m_pDoc = pDoc; m_pRPC_Thread_Mutex = pRPC_Thread_Mutex; m_pRPC_Thread_Condition = pRPC_Thread_Condition; + m_pRPC_Request_Mutex = pRPC_Request_Mutex; + m_pRPC_Request_Condition = pRPC_Request_Condition; } @@ -116,6 +125,7 @@ void *RPCThread::Entry() { int retval = 0; CRPCFinishedEvent RPC_done_event( wxEVT_RPC_FINISHED ); ASYNC_RPC_REQUEST *current_request; + wxMutexError mutexErr = wxMUTEX_NO_ERROR; wxCondError condErr = wxCOND_NO_ERROR; m_pRPC_Thread_Mutex->Lock(); @@ -123,8 +133,8 @@ void *RPCThread::Entry() { while(true) { // Wait for main thread to wake us // This does the following: - // (1) Unlocks the Mutex an puts the thread to sleep as an atomic operation. - // (2) On Signal from main thread, wakes and then automatically locks m_pRPC_Thread_Mutex again. + // (1) Unlocks the Mutex and puts the RPC thread to sleep as an atomic operation. + // (2) On Signal from main thread: locks Mutex again and wakes the RPC thread. if (!m_pDoc->m_bShutDownRPCThread) { condErr = m_pRPC_Thread_Condition->Wait(); wxASSERT(condErr == wxCOND_NO_ERROR); @@ -154,9 +164,19 @@ void *RPCThread::Entry() { retval = ProcessRPCRequest(); current_request->retval = retval; - current_request->isActive = false; + mutexErr = m_pRPC_Request_Mutex->Lock(); + wxASSERT(mutexErr == wxMUTEX_NO_ERROR); + + current_request->isActive = false; wxPostEvent( wxTheApp, RPC_done_event ); + + // Signal() is ignored / discarded unless the main thread is + // currently blocked by m_pRPC_Request_Condition->Wait[Timeout]() + m_pRPC_Request_Condition->Signal(); + + mutexErr = m_pRPC_Request_Mutex->Unlock(); + wxASSERT(mutexErr == wxMUTEX_NO_ERROR); } return NULL; @@ -401,8 +421,11 @@ int CMainDocument::RequestRPC(ASYNC_RPC_REQUEST& request, bool hasPriority) { std::vector::iterator iter; int retval = 0; wxMutexError mutexErr = wxMUTEX_NO_ERROR; - bool keepLooping = true; + long delayTimeRemaining, timeToSleep; + bool shown = false; + if (!m_RPCThread) return -1; + if ( (request.rpcType < RPC_TYPE_WAIT_FOR_COMPLETION) || (request.rpcType >= NUM_RPC_TYPES) ) { wxASSERT(false); @@ -445,10 +468,10 @@ int CMainDocument::RequestRPC(ASYNC_RPC_REQUEST& request, bool hasPriority) { mutexErr = m_pRPC_Thread_Mutex->Lock(); // Blocks until thread unlocks the mutex wxASSERT(mutexErr == wxMUTEX_NO_ERROR); + m_pRPC_Thread_Condition->Signal(); // Unblock the thread + mutexErr = m_pRPC_Thread_Mutex->Unlock(); // Release the mutex so thread can lock it wxASSERT(mutexErr == wxMUTEX_NO_ERROR); - - m_pRPC_Thread_Condition->Signal(); // Unblock the thread } // If this is a user-initiated event wait for completion but show @@ -469,48 +492,62 @@ int CMainDocument::RequestRPC(ASYNC_RPC_REQUEST& request, bool hasPriority) { // Allow RPC_WAIT_DLG_DELAY seconds for Demand RPC to complete before // displaying "Please Wait" dialog, but keep checking for completion. - do { -#ifdef __WXMSW__ - SwitchToThread(); -#else - // TODO: is there a way for main UNIX thread to yield wih no minimum delay? - timespec ts = {0, 1}; /// 1 nanosecond - nanosleep(&ts, NULL); /// 1 nanosecond or less -#endif + delayTimeRemaining = RPC_WAIT_DLG_DELAY; + while (true) { + if (delayTimeRemaining >= 0) { // Prevent overflow if minimized for a very long time + delayTimeRemaining = RPC_WAIT_DLG_DELAY - Dlgdelay.Time(); + } + + if (pFrame) { + shown = pFrame->IsShown(); + } else { + shown = false; + } + + if (shown) { + if (delayTimeRemaining <= 0) break; // Display the Please Wait dialog + timeToSleep = delayTimeRemaining; + } else { + // Don't show dialog while Manager is minimized, but do + // process events so user can maximize the manager. + // + // NOTE: CBOINCGUIApp::FilterEvent() blocks those events + // which might cause posting of more RPC requests while + // we are in this loop, to prevent undesirable recursion. + // It does allow wxEVT_RPC_FINISHED. + // + timeToSleep = DELAY_WHEN_MINIMIZED; // Allow user to maximize Manager + wxSafeYield(NULL, true); + } + // OnRPCComplete() clears m_bWaitingForRPC if RPC completed if (! m_bWaitingForRPC) { return retval; } - keepLooping = (Dlgdelay.Time() < RPC_WAIT_DLG_DELAY); - if (keepLooping) { - // Simulate handling of CRPCFinishedEvent but don't allow any other - // events (so no user activity) to prevent undesirable recursion. - // Allow RPC thread to run while we wait for it. - if (!current_rpc_request.isActive) { - HandleCompletedRPC(); - } - } else { - // RPC_WAIT_DLG_DELAY has expired; check if Manager is minimized. - if (pFrame) { - if (!pFrame->IsShown()) { - // Don't show dialog while Manager is minimized, but do - // process events so user can maximize the manager. - // - // NOTE: CBOINCGUIApp::FilterEvent() blocks those events - // which might cause posting of more RPC requests while - // we are in this loop, to prevent undesirable recursion. - // It does allow wxEVT_RPC_FINISHED so we don't need to - // explicity call HandleCompletedRPC() here. - // - keepLooping = true; - if (wxGetApp().Pending()) { - wxGetApp().Dispatch(); - } - } - } + mutexErr = m_pRPC_Request_Mutex->Lock(); + wxASSERT(mutexErr == wxMUTEX_NO_ERROR); + + // Simulate handling of CRPCFinishedEvent but don't allow any other + // events (so no user activity) to prevent undesirable recursion. + // Allow RPC thread to run while we wait for it. + if (!current_rpc_request.isActive) { + mutexErr = m_pRPC_Request_Mutex->Unlock(); + wxASSERT(mutexErr == wxMUTEX_NO_ERROR); + + HandleCompletedRPC(); + continue; } - } while (keepLooping); + + // Wait for RPC thread to wake us + // This does the following: + // (1) Unlocks the Mutex and puts the main thread to sleep as an atomic operation. + // (2) On Signal from RPC thread: locks Mutex again and wakes the main thread. + m_pRPC_Request_Condition->WaitTimeout(timeToSleep); + + mutexErr = m_pRPC_Request_Mutex->Unlock(); + wxASSERT(mutexErr == wxMUTEX_NO_ERROR); + } // Demand RPC has taken longer than RPC_WAIT_DLG_DELAY seconds and // Manager is not minimized, so display the "Please Wait" dialog @@ -568,10 +605,9 @@ void CMainDocument::KillRPCThread() { } rpcClient.close(); - RPC_requests.clear(); - current_rpc_request.clear(); m_bNeedRefresh = false; m_bNeedTaskBarRefresh = false; + m_bShutDownRPCThread = true; // On some platforms, Delete() takes effect only when thread calls TestDestroy() @@ -583,6 +619,10 @@ void CMainDocument::KillRPCThread() { wxASSERT(mutexErr == wxMUTEX_NO_ERROR); m_pRPC_Thread_Condition->Signal(); // Unblock the thread + m_RPCThread->Delete(); + + RPC_requests.clear(); + current_rpc_request.clear(); wxStopWatch ThreadDeleteTimer = wxStopWatch(); // RPC thread sets m_RPCThread to NULL when it exits @@ -606,6 +646,8 @@ void CMainDocument::HandleCompletedRPC() { wxMutexError mutexErr = wxMUTEX_NO_ERROR; int i, n, requestIndex = -1; bool stillWaitingForPendingRequests = false; + + if (!m_RPCThread) return; if (current_rpc_request.isActive) return; diff --git a/clientgui/AsyncRPC.h b/clientgui/AsyncRPC.h index db4ffbd129..3a0f788225 100644 --- a/clientgui/AsyncRPC.h +++ b/clientgui/AsyncRPC.h @@ -287,7 +287,12 @@ private: class RPCThread : public wxThread { public: - RPCThread(CMainDocument *pDoc, wxMutex* pRPC_Thread_Mutex, wxCondition* pRPC_Thread_Condition); + RPCThread(CMainDocument *pDoc, + wxMutex* pRPC_Thread_Mutex, + wxCondition* pRPC_Thread_Condition, + wxMutex* pRPC_Request_Mutex, + wxCondition* RPC_Request_Condition + ); virtual void *Entry(); virtual void OnExit(); @@ -296,6 +301,8 @@ private: CMainDocument* m_pDoc; wxMutex* m_pRPC_Thread_Mutex; wxCondition* m_pRPC_Thread_Condition; + wxMutex* m_pRPC_Request_Mutex; + wxCondition* m_pRPC_Request_Condition; }; diff --git a/clientgui/BOINCBaseFrame.cpp b/clientgui/BOINCBaseFrame.cpp index fe3eb65f3a..75b75cd557 100644 --- a/clientgui/BOINCBaseFrame.cpp +++ b/clientgui/BOINCBaseFrame.cpp @@ -349,17 +349,12 @@ void CBOINCBaseFrame::OnExit(wxCommandEvent& WXUNUSED(event)) { // Under wxWidgets 2.8.0, the task bar icons must be deleted for app to exit its main loop #ifdef __WXMAC__ - CMacSystemMenu* pMSM = wxGetApp().GetMacSystemMenu(); - if (pMSM) - delete pMSM; + wxGetApp().DeleteMacSystemMenu(); #endif // TaskBarIcon isn't used in Linux #if defined(__WXMSW__) || defined(__WXMAC__) - CTaskBarIcon* pTBI = wxGetApp().GetTaskBarIcon(); - if (pTBI && !pTBI->m_bTaskbarInitiatedShutdown) { - delete pTBI; - } + wxGetApp().DeleteTaskBarIcon(); #endif Close(true); } diff --git a/clientgui/BOINCGUIApp.cpp b/clientgui/BOINCGUIApp.cpp index f0dafac6e4..4d1696b944 100644 --- a/clientgui/BOINCGUIApp.cpp +++ b/clientgui/BOINCGUIApp.cpp @@ -971,6 +971,24 @@ bool CBOINCGUIApp::IsModalDialogDisplayed() { } +void CBOINCGUIApp::DeleteTaskBarIcon() { + if (m_pTaskBarIcon) { + delete m_pTaskBarIcon; + } + m_pTaskBarIcon = NULL; +} + + +#ifdef __WXMAC__ +void CBOINCGUIApp::DeleteMacSystemMenu() { + if (m_pMacSystemMenu) { + delete m_pMacSystemMenu; + } + m_pMacSystemMenu = NULL; +} +#endif + + // Prevent recursive entry of CMainDocument::RequestRPC() int CBOINCGUIApp::FilterEvent(wxEvent &event) { if (!m_pDocument) return -1; diff --git a/clientgui/BOINCGUIApp.h b/clientgui/BOINCGUIApp.h index 37ec273cd6..7c8da6af93 100644 --- a/clientgui/BOINCGUIApp.h +++ b/clientgui/BOINCGUIApp.h @@ -127,9 +127,11 @@ public: wxString GetDataDirectory() { return m_strBOINCMGRDataDirectory; } #if defined(__WXMSW__) || defined(__WXMAC__) CTaskBarIcon* GetTaskBarIcon() { return m_pTaskBarIcon; } + void DeleteTaskBarIcon(); #endif #ifdef __WXMAC__ CMacSystemMenu* GetMacSystemMenu() { return m_pMacSystemMenu; } + void DeleteMacSystemMenu(); int ShouldShutdownCoreClient() { return true; } #else diff --git a/clientgui/MainDocument.cpp b/clientgui/MainDocument.cpp index a521f4ab03..a7480fe803 100644 --- a/clientgui/MainDocument.cpp +++ b/clientgui/MainDocument.cpp @@ -443,8 +443,20 @@ int CMainDocument::OnInit() { wxASSERT(m_pRPC_Thread_Mutex); m_pRPC_Thread_Condition = new wxCondition(*m_pRPC_Thread_Mutex); - - m_RPCThread = new RPCThread(this, m_pRPC_Thread_Mutex, m_pRPC_Thread_Condition); + wxASSERT(m_pRPC_Thread_Condition); + + m_pRPC_Request_Mutex = new wxMutex(); + wxASSERT(m_pRPC_Request_Mutex); + + m_pRPC_Request_Condition = new wxCondition(*m_pRPC_Request_Mutex); + wxASSERT(m_pRPC_Request_Condition); + + m_RPCThread = new RPCThread(this, + m_pRPC_Thread_Mutex, + m_pRPC_Thread_Condition, + m_pRPC_Request_Mutex, + m_pRPC_Request_Condition + ); wxASSERT(m_RPCThread); iRetVal = m_RPCThread->Create(); @@ -459,9 +471,6 @@ int CMainDocument::OnInit() { int CMainDocument::OnExit() { int iRetVal = 0; - RPC_requests.clear(); - current_rpc_request.clear(); - if (m_pClientManager) { m_pClientManager->ShutdownBOINCCore(); @@ -480,6 +489,12 @@ int CMainDocument::OnExit() { delete m_pRPC_Thread_Condition; m_pRPC_Thread_Condition = NULL; + delete m_pRPC_Request_Mutex; + m_pRPC_Request_Mutex = NULL; + + delete m_pRPC_Request_Condition; + m_pRPC_Request_Condition = NULL; + rpcClient.close(); if (m_pNetworkConnection) { diff --git a/clientgui/MainDocument.h b/clientgui/MainDocument.h index d12e7a89a6..560d868aec 100644 --- a/clientgui/MainDocument.h +++ b/clientgui/MainDocument.h @@ -182,7 +182,6 @@ public: wxDialog* GetRPCWaitDialog() { return m_RPCWaitDlg; } // void TestAsyncRPC(); // For testing Async RPCs RPCThread* m_RPCThread; - wxCriticalSection m_critsect; bool m_bShutDownRPCThread; private: @@ -197,6 +196,8 @@ private: bool m_bNeedTaskBarRefresh; wxMutex* m_pRPC_Thread_Mutex; wxCondition* m_pRPC_Thread_Condition; + wxMutex* m_pRPC_Request_Mutex; + wxCondition* m_pRPC_Request_Condition; // // Project Tab diff --git a/clientgui/mac/MacSysMenu.cpp b/clientgui/mac/MacSysMenu.cpp index d0a055f31f..7615864124 100644 --- a/clientgui/mac/MacSysMenu.cpp +++ b/clientgui/mac/MacSysMenu.cpp @@ -299,6 +299,7 @@ pascal OSStatus SysMenuEventHandler( EventHandlerCallRef inHandlerCallRef, return eventNotHandledErr; pMSM = wxGetApp().GetMacSystemMenu(); + if (!pMSM) break; // wxMac "helpfully" converts wxID_ABOUT to kHICommandAbout, wxID_EXIT to kHICommandQuit, // wxID_PREFERENCES to kHICommandPreferences @@ -352,10 +353,8 @@ pascal OSStatus SysMenuEventHandler( EventHandlerCallRef inHandlerCallRef, // Note that if the main window is open, CBOINCBaseFrame::OnExit() will be // called and SysMenuEventHandler() (i.e., this code) will not be called. if (commandID == wxID_EXIT) { - delete pMSM; - CTaskBarIcon* pTBI = wxGetApp().GetTaskBarIcon(); - if (pTBI) - delete pTBI; + wxGetApp().DeleteMacSystemMenu(); + wxGetApp().DeleteTaskBarIcon(); } return noErr ; }