diff --git a/clientgui/MainDocument.cpp b/clientgui/MainDocument.cpp index 06e447fd18..8697996c2c 100644 --- a/clientgui/MainDocument.cpp +++ b/clientgui/MainDocument.cpp @@ -1,6 +1,6 @@ // This file is part of BOINC. // http://boinc.berkeley.edu -// Copyright (C) 2022 University of California +// Copyright (C) 2023 University of California // // BOINC is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License @@ -1561,7 +1561,7 @@ int CMainDocument::GetWorkCount() { } -int CMainDocument::WorkSuspend(char* url, char* name) { +int CMainDocument::WorkSuspend(const char* url, const char* name) { int iRetVal = 0; RESULT* pStateResult = state.lookup_result(url, name); @@ -1575,7 +1575,7 @@ int CMainDocument::WorkSuspend(char* url, char* name) { } -int CMainDocument::WorkResume(char* url, char* name) { +int CMainDocument::WorkResume(const char* url, const char* name) { int iRetVal = 0; RESULT* pStateResult = state.lookup_result(url, name); @@ -1984,7 +1984,7 @@ int CMainDocument::WorkShowVMConsole(RESULT* res) { } -int CMainDocument::WorkAbort(char* url, char* name) { +int CMainDocument::WorkAbort(const char* url, const char* name) { int iRetVal = 0; RESULT* pStateResult = state.lookup_result(url, name); diff --git a/clientgui/MainDocument.h b/clientgui/MainDocument.h index f72bce98b4..56cbdef0bf 100644 --- a/clientgui/MainDocument.h +++ b/clientgui/MainDocument.h @@ -1,6 +1,6 @@ // This file is part of BOINC. // http://boinc.berkeley.edu -// Copyright (C) 2022 University of California +// Copyright (C) 2023 University of California // // BOINC is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License @@ -279,11 +279,11 @@ public: int GetWorkCount(); - int WorkSuspend(char* url, char* name); - int WorkResume(char* url, char* name); + int WorkSuspend(const char* url, const char* name); + int WorkResume(const char* url, const char* name); int WorkShowGraphics(RESULT* result); int WorkShowVMConsole(RESULT* result); - int WorkAbort(char* url, char* name); + int WorkAbort(const char* url, const char* name); CC_STATE* GetState() { return &state; }; diff --git a/clientgui/ViewWork.cpp b/clientgui/ViewWork.cpp index 35bba6e0e5..5fa81fe596 100644 --- a/clientgui/ViewWork.cpp +++ b/clientgui/ViewWork.cpp @@ -1,6 +1,6 @@ // This file is part of BOINC. // http://boinc.berkeley.edu -// Copyright (C) 2022 University of California +// Copyright (C) 2023 University of California // // BOINC is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License @@ -462,6 +462,15 @@ void CViewWork::OnActiveTasksOnly( wxCommandEvent& WXUNUSED(event) ) { } +// Comparator functions for sort operations in OnWorkSuspend() and OnWorkAbort() +static bool sort_order_started(const RESULT* a, const RESULT* b) { + return !a->is_not_started() && b->is_not_started(); +} +static bool sort_order_not_started(const RESULT* a, const RESULT* b) { + return a->is_not_started() && !b->is_not_started(); +} + + void CViewWork::OnWorkSuspend( wxCommandEvent& WXUNUSED(event) ) { wxLogTrace(wxT("Function Start/End"), wxT("CViewWork::OnWorkSuspend - Function Begin")); @@ -476,6 +485,13 @@ void CViewWork::OnWorkSuspend( wxCommandEvent& WXUNUSED(event) ) { wxASSERT(m_pTaskPane); wxASSERT(m_pListPane); + // It shouldn't be possible to get in here with a mixture of suspended and non- + // suspended tasks selected, so one of these lists should always end up empty. + // It is not possible for any one task to end up in both lists. + std::vector results_to_resume; + std::vector results_to_suspend; + + // Collect the selected tasks, without actioning the request yet row = -1; while (1) { // Step through all selected items @@ -485,13 +501,41 @@ void CViewWork::OnWorkSuspend( wxCommandEvent& WXUNUSED(event) ) { RESULT* result = pDoc->result(m_iSortedIndexes[row]); if (result) { if (result->suspended_via_gui) { - pDoc->WorkResume(result->project_url, result->name); + results_to_resume.push_back(result); } else { - pDoc->WorkSuspend(result->project_url, result->name); + results_to_suspend.push_back(result); } } } + // Sort + // + // Until/unless the client can accept requests to operate on a batch of tasks at once + // (and thus make its own decision about the order in which to perform the actions), + // sort the lists to avoid unwanted side-effects of the client asynchronously scheduling tasks. + // In particular this takes care of the case where the user wants to suspend all tasks, + // some of which are running and some of which have not yet started. Without this sorting, + // suspending the first running task will open up a slot to the client, which will immediately + // find a ready task and start it running, not knowing that we also want to suspend that one. + // Resume is the opposite: resume tasks that have already started before those that have not. + // Otherwise, the user probably expects to see requests take effect in the same order as the + // selection in the GUI - so use a stable sort. + std::stable_sort(results_to_resume.begin(), results_to_resume.end(), sort_order_started); + std::stable_sort(results_to_suspend.begin(), results_to_suspend.end(), sort_order_not_started); + + // Do the action(s) + // + // One of these lists should always be empty, so the order in which we action + // the two makes no difference. If somehow we do end up with entries in both, + // sending the resume requests first gives the client a more up-to-date picture + // before the suspensions cause it to go looking for tasks that are ready to run. + for (const RESULT* result : results_to_resume) { + pDoc->WorkResume(result->project_url, result->name); + } + for (const RESULT* result : results_to_suspend) { + pDoc->WorkSuspend(result->project_url, result->name); + } + UpdateSelection(); pFrame->FireRefreshView(); @@ -615,6 +659,8 @@ void CViewWork::OnWorkAbort( wxCommandEvent& WXUNUSED(event) ) { return; } + std::vector results_to_abort; + row = -1; while (1) { // Step through all selected items @@ -623,10 +669,18 @@ void CViewWork::OnWorkAbort( wxCommandEvent& WXUNUSED(event) ) { RESULT* result = pDoc->result(m_iSortedIndexes[row]); if (result) { - pDoc->WorkAbort(result->project_url, result->name); + results_to_abort.push_back(result); } } + // Abort tasks that have not yet started before those that have. + // See longer explanation in OnWorkSuspend() + std::stable_sort(results_to_abort.begin(), results_to_abort.end(), sort_order_not_started); + + for (const RESULT* result : results_to_abort) { + pDoc->WorkAbort(result->project_url, result->name); + } + UpdateSelection(); pFrame->FireRefreshView(); diff --git a/lib/gui_rpc_client.h b/lib/gui_rpc_client.h index 990b3c1bb6..236be2b0a8 100644 --- a/lib/gui_rpc_client.h +++ b/lib/gui_rpc_client.h @@ -1,6 +1,6 @@ // This file is part of BOINC. // https://boinc.berkeley.edu -// Copyright (C) 2022 University of California +// Copyright (C) 2023 University of California // // BOINC is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License @@ -311,6 +311,13 @@ struct RESULT { int parse(XML_PARSER&); void print(); void clear(); + + bool is_not_started() const { + if (state >= RESULT_COMPUTE_ERROR) return false; + if (ready_to_report) return false; + if (active_task) return false; + return true; + } }; struct FILE_TRANSFER {