From 3e05e9327878cbe0bb4a599886ccfbe2926d5913 Mon Sep 17 00:00:00 2001 From: Charlie Fenton Date: Tue, 23 Sep 2008 00:16:43 +0000 Subject: [PATCH] MGR: Simplify Async GUI RPC refresh event logic; prevent a possible case of undesirable recursion. svn path=/trunk/boinc/; revision=16040 --- clientgui/AsyncRPC.cpp | 72 +++++++++++++++++++++----------------- clientgui/AsyncRPC.h | 28 ++++++++------- clientgui/MainDocument.cpp | 40 +++++++-------------- clientgui/MainDocument.h | 1 + 4 files changed, 69 insertions(+), 72 deletions(-) diff --git a/clientgui/AsyncRPC.cpp b/clientgui/AsyncRPC.cpp index ed6ba13008..0f7e14086b 100755 --- a/clientgui/AsyncRPC.cpp +++ b/clientgui/AsyncRPC.cpp @@ -47,10 +47,10 @@ void ASYNC_RPC_REQUEST::clear() { arg2 = NULL; arg3 = NULL; arg4 = NULL; - event = NULL; - eventHandler = NULL; + rpcType = (ASYNC_RPC_TYPE) 0; completionTime = NULL; resultPtr = NULL; + retval = 0; isActive = false; } @@ -62,15 +62,10 @@ bool ASYNC_RPC_REQUEST::isSameAs(ASYNC_RPC_REQUEST& otherRequest) { if (arg2 != otherRequest.arg2) return false; if (arg3 != otherRequest.arg3) return false; if (arg4 != otherRequest.arg4) return false; - if (event != otherRequest.event) { - if (event->GetEventType() != (otherRequest.event)->GetEventType()) return false; - if (event->GetId() != (otherRequest.event)->GetId()) return false; - if (event->GetEventObject() != (otherRequest.event)->GetEventObject()) return false; - } - if (eventHandler != otherRequest.eventHandler) return false; + if (rpcType != otherRequest.rpcType) return false; if (completionTime != otherRequest.completionTime) return false; if (resultPtr != otherRequest.resultPtr) return false; - // OK if isActive doesn't match. + // OK if isActive and retval don't match. return true; } @@ -94,6 +89,7 @@ int AsyncRPC::RPC_Wait(RPC_SELECTOR which_rpc, void *arg1, void *arg2, request.arg2 = arg2; request.arg3 = arg3; request.arg4 = arg4; + request.rpcType = RPC_TYPE_WAIT_FOR_COMPLETION; retval = m_Doc->RequestRPC(request, hasPriority); return retval; @@ -414,6 +410,12 @@ int CMainDocument::RequestRPC(ASYNC_RPC_REQUEST& request, bool hasPriority) { std::vector::iterator iter; int retval = 0, retval2 = 0; + if ( (request.rpcType < RPC_TYPE_WAIT_FOR_COMPLETION) || + (request.rpcType >= NUM_RPC_TYPES) ) { + wxASSERT(false); + return -1; + } + // If we are quitting, cancel any pending RPCs if (request.which_rpc == RPC_QUIT) { RPC_requests.clear(); @@ -426,7 +428,7 @@ int CMainDocument::RequestRPC(ASYNC_RPC_REQUEST& request, bool hasPriority) { } } - if ((request.event == 0) && (request.resultPtr == NULL)) { + if ((request.rpcType == RPC_TYPE_WAIT_FOR_COMPLETION) && (request.resultPtr == NULL)) { request.resultPtr = &retval; } @@ -452,9 +454,9 @@ int CMainDocument::RequestRPC(ASYNC_RPC_REQUEST& request, bool hasPriority) { } #endif // !!__WXMSW__ // Deadlocks on Windows - // If no completion event specified, this is a user-initiated event so - // wait for completion but show a dialog allowing the user to cancel. - if (request.event == 0) { + // If this is a user-initiated event wait for completion but show + // a dialog allowing the user to cancel. + if (request.rpcType == RPC_TYPE_WAIT_FOR_COMPLETION) { // TODO: proper handling if a second user request is received while first is pending ?? if (m_bWaitingForRPC) { wxLogMessage(wxT("Second user RPC request while another was pending")); @@ -564,7 +566,7 @@ void CMainDocument::HandleCompletedRPC() { if (RPC_requests[i].isSameAs(current_rpc_request)) { requestIndex = i; } else { - if (RPC_requests[i].event == 0) { + if (RPC_requests[i].rpcType == RPC_TYPE_WAIT_FOR_COMPLETION) { stillWaitingForPendingRequests = true; } } @@ -583,7 +585,6 @@ void CMainDocument::HandleCompletedRPC() { if (requestIndex >= 0) { // Remove completed request from the queue - RPC_requests[requestIndex].event = NULL; // Is this needed to prevent calling the event's destructor? RPC_requests.erase(RPC_requests.begin()+requestIndex); } @@ -596,10 +597,6 @@ void CMainDocument::HandleCompletedRPC() { *(current_rpc_request.resultPtr) = retval; } - // We must get the frame immediately before using it, - // since it may have been changed by SetActiveGUI(). - CBOINCBaseFrame* pFrame = wxGetApp().GetFrame(); - // Post-processing if (! retval) { switch (current_rpc_request.which_rpc) { @@ -714,6 +711,12 @@ void CMainDocument::HandleCompletedRPC() { } } + if (current_rpc_request.rpcType == RPC_TYPE_ASYNC_WITH_REFRESH_AFTER) { + if (!retval) { + m_bNeedRefresh = true; + } + } + // We must call ProcessEvent() rather than AddPendingEvent() here to // guarantee integrity of data when other events are handled (such as // Abort, Suspend/Resume, Show Graphics, Update, Detach, Reset, No @@ -723,19 +726,23 @@ void CMainDocument::HandleCompletedRPC() { // deleted due to the RPC. // The refresh event called here adjusts the selections to fix any // such mismatch before other pending events are processed. - if ( (current_rpc_request.event) && (current_rpc_request.event != (wxEvent*)-1) ) { - if (!retval) { - if (current_rpc_request.eventHandler) { - current_rpc_request.eventHandler->ProcessEvent(*current_rpc_request.event); - } else { - if (pFrame) { - wxASSERT(wxDynamicCast(pFrame, CBOINCBaseFrame)); - pFrame->ProcessEvent(*current_rpc_request.event); - } - } + // + // However, the refresh code may itself request a Demand RPC, which + // would cause undesirable recursion if we are already waiting for + // another Demand RPC to complete. In that case, we defer the refresh + // until all pending Demand RPCs have been done. + // + if (m_bNeedRefresh && !m_bWaitingForRPC) { + m_bNeedRefresh = false; + // We must get the frame immediately before using it, + // since it may have been changed by SetActiveGUI(). + CBOINCBaseFrame* pFrame = wxGetApp().GetFrame(); + + wxASSERT(wxDynamicCast(pFrame, CBOINCBaseFrame)); + if (pFrame) { + CFrameEvent event(wxEVT_FRAME_REFRESHVIEW, pFrame); + pFrame->ProcessEvent(event); } - delete current_rpc_request.event; - current_rpc_request.event = NULL; } current_rpc_request.clear(); @@ -814,8 +821,7 @@ void CMainDocument::TestAsyncRPC() { request.arg2 = NULL; request.arg3 = NULL; request.arg4 = NULL; - request.event = NULL; - request.eventHandler = NULL; + request.rpcType = RPC_TYPE_WAIT_FOR_COMPLETION; request.completionTime = &completionTime; // request.result = NULL; request.resultPtr = &rpc_result; // For testing async RPCs diff --git a/clientgui/AsyncRPC.h b/clientgui/AsyncRPC.h index fb0eaed143..85eba4e85f 100644 --- a/clientgui/AsyncRPC.h +++ b/clientgui/AsyncRPC.h @@ -89,6 +89,20 @@ enum RPC_SELECTOR { }; +enum ASYNC_RPC_TYPE { + // Demand RPC: wait for completion before returning (usually + // a user-initiated request.) + RPC_TYPE_WAIT_FOR_COMPLETION = 1, + // Periodic RPC: post request on queue and return immediately + // (requested due to a timer interrupt.) + RPC_TYPE_ASYNC_NO_REFRESH, + // Periodic RPCas above, but on completion also process a + // wxEVT_FRAME_REFRESHVIEW event to refresh the display. + RPC_TYPE_ASYNC_WITH_REFRESH_AFTER, + NUM_RPC_TYPES + +}; + // Pass the following structure to CMainDocument::RequestRPC() // The members are as follows: // @@ -101,16 +115,7 @@ enum RPC_SELECTOR { // arg2, arg3, arg4 are additional arguments when needed by the // RPC call; their usage varies for different RPC requests. // -// event is an (optional) event to be posted on completion of a -// periodic RPC. If a periodic RPC should not post an event -// on completion, set this to (wxEvent*)-1. A NULL value -// indicates a user-initiated (demand) RPC request; the call -// will not return until the RPC completes. -// -// eventHandler is the eventhandler to which to displatch the -// completion event (typically a wxWindow, wxFrame, or wxApp). -// If it is NULL, the current CBOINCBaseFrame is used. It is -// ignored if the event parameter is NULL or -1. +// rpcType is as described above // // completionTime is a pointer to a wxDateTime variable into which // to write the completion time of the RPC. It may be NULL. @@ -130,8 +135,7 @@ struct ASYNC_RPC_REQUEST { void *arg2; void *arg3; void *arg4; - wxEvent *event; - wxEvtHandler *eventHandler; + ASYNC_RPC_TYPE rpcType; wxDateTime *completionTime; int *resultPtr; int retval; diff --git a/clientgui/MainDocument.cpp b/clientgui/MainDocument.cpp index 0f0eb1dbae..191919dc2b 100644 --- a/clientgui/MainDocument.cpp +++ b/clientgui/MainDocument.cpp @@ -401,6 +401,7 @@ int CMainDocument::OnInit() { m_RPCWaitDlg = NULL; m_bWaitingForRPC = false; + m_bNeedRefresh = false; current_rpc_request.clear(); m_RPCThread = new RPCThread(this); @@ -747,8 +748,7 @@ void CMainDocument::RunPeriodicRPCs() { // or project in a demand RPC call. If Periodic RPCs continue // to run during these dialogs, that pointer may no longer be // valid by the time the demand RPC is executed. So we suspend - // periodic RPCs during modal dialogs. Search for the dialog - // by ID since all of BOINC Manager's dialog IDs are 10000. + // periodic RPCs during modal dialogs. // // Note that this depends on using wxGetApp().SafeMessageBox() // instead of wxMessageBox in all tab views. @@ -767,7 +767,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_CC_STATUS; request.arg1 = &async_status_buf; request.exchangeBuf = &status; - request.event = (wxEvent*)-1; + request.rpcType = RPC_TYPE_ASYNC_NO_REFRESH; request.completionTime = &m_dtCachedCCStatusTimestamp; request.resultPtr = &m_iGet_status_rpc_result; @@ -783,7 +783,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_STATE; request.arg1 = &async_state_buf; request.exchangeBuf = &state; - request.event = (wxEvent*)-1; + request.rpcType = RPC_TYPE_ASYNC_NO_REFRESH; request.resultPtr = &m_iGet_state_rpc_result; RequestRPC(request); @@ -794,7 +794,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_HOST_INFO; request.arg1 = &async_host_buf; request.exchangeBuf = &host; - request.event = (wxEvent*)-1; + request.rpcType = RPC_TYPE_ASYNC_NO_REFRESH; request.completionTime = &m_dtCachedStateTimestamp; request.resultPtr = &m_iGet_host_info_rpc_result; @@ -810,9 +810,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_PROJECT_STATUS1; request.arg1 = &async_state_buf; request.exchangeBuf = &state; - request.event = new CFrameEvent(wxEVT_FRAME_REFRESHVIEW, pFrame); - // NULL request.eventHandler means use CBOINCBaseFrame when RPC has - // finished, which may have changed since request was made + request.rpcType = RPC_TYPE_ASYNC_WITH_REFRESH_AFTER; request.completionTime = &m_dtProjecStatusTimestamp; request.resultPtr = &m_iGet_project_status1_rpc_result; @@ -829,9 +827,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_RESULTS; request.arg1 = &async_results_buf; request.exchangeBuf = &results; - request.event = new CFrameEvent(wxEVT_FRAME_REFRESHVIEW, pFrame); - // NULL request.eventHandler means use CBOINCBaseFrame when RPC has - // finished, which may have changed since request was made + request.rpcType = RPC_TYPE_ASYNC_WITH_REFRESH_AFTER; request.completionTime = &m_dtResultsTimestamp; request.resultPtr = &m_iGet_results_rpc_result; @@ -848,9 +844,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_FILE_TRANSFERS; request.arg1 = &async_ft_buf; request.exchangeBuf = &ft; - request.event = new CFrameEvent(wxEVT_FRAME_REFRESHVIEW, pFrame); - // NULL request.eventHandler means use CBOINCBaseFrame when RPC has - // finished, which may have changed since request was made + request.rpcType = RPC_TYPE_ASYNC_WITH_REFRESH_AFTER; request.completionTime = &m_dtFileTransfersTimestamp; request.resultPtr = &m_iGet_file_transfers_rpc_result; @@ -869,9 +863,7 @@ void CMainDocument::RunPeriodicRPCs() { request.arg2 = &messages; // request.arg2 = &async_messages_buf; // request.exchangeBuf = &messages; - request.event = new CFrameEvent(wxEVT_FRAME_REFRESHVIEW, pFrame); - // NULL request.eventHandler means use CBOINCBaseFrame when RPC has - // finished, which may have changed since request was made + request.rpcType = RPC_TYPE_ASYNC_WITH_REFRESH_AFTER; request.completionTime = NULL; request.resultPtr = &m_iGet_messages_rpc_result; @@ -887,9 +879,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_STATISTICS; request.arg1 = &async_statistics_status_buf; request.exchangeBuf = &statistics_status; - request.event = new CFrameEvent(wxEVT_FRAME_REFRESHVIEW, pFrame); - // NULL request.eventHandler means use CBOINCBaseFrame when RPC has - // finished, which may have changed since request was made + request.rpcType = RPC_TYPE_ASYNC_WITH_REFRESH_AFTER; request.completionTime = &m_dtStatisticsStatusTimestamp; request.resultPtr = &m_iGet_statistics_rpc_result; @@ -906,9 +896,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_GET_DISK_USAGE; request.arg1 = &async_disk_usage_buf; request.exchangeBuf = &disk_usage; - request.event = new CFrameEvent(wxEVT_FRAME_REFRESHVIEW, pFrame); - // NULL request.eventHandler means use CBOINCBaseFrame when RPC has - // finished, which may have changed since request was made + request.rpcType = RPC_TYPE_ASYNC_WITH_REFRESH_AFTER; request.completionTime = &m_dtDiskUsageTimestamp; request.resultPtr = &m_iGet_dsk_usage_rpc_result; @@ -927,9 +915,7 @@ void CMainDocument::RunPeriodicRPCs() { request.exchangeBuf = &state; request.arg2 = &async_results_buf; request.arg3 = &results; - request.event = new CFrameEvent(wxEVT_FRAME_REFRESHVIEW, pFrame); - // NULL request.eventHandler means use CBOINCBaseFrame when RPC has - // finished, which may have changed since request was made + request.rpcType = RPC_TYPE_ASYNC_WITH_REFRESH_AFTER; request.completionTime = &m_dtCachedSimpleGUITimestamp; request.resultPtr = &m_iGet_simple_gui2_rpc_result; @@ -945,7 +931,7 @@ void CMainDocument::RunPeriodicRPCs() { request.which_rpc = RPC_ACCT_MGR_INFO; request.arg1 = &async_ami_buf; request.exchangeBuf = &ami; - request.event = (wxEvent*)-1; + request.rpcType = RPC_TYPE_ASYNC_NO_REFRESH; request.completionTime = &m_dtCachedAcctMgrInfoTimestamp; request.resultPtr = &m_iAcct_mgr_info_rpc_result; diff --git a/clientgui/MainDocument.h b/clientgui/MainDocument.h index 1958f7bc32..53f374f185 100644 --- a/clientgui/MainDocument.h +++ b/clientgui/MainDocument.h @@ -181,6 +181,7 @@ public: // void TestAsyncRPC(); // For testing Async RPCs RPCThread* m_RPCThread; wxCriticalSection m_critsect; + bool m_bNeedRefresh; private: ASYNC_RPC_REQUEST current_rpc_request;