From 49a9ac54d5acf6fb4fe83ed6e08696f65d61dcc5 Mon Sep 17 00:00:00 2001 From: hiker Date: Wed, 4 Jun 2014 17:05:35 +1000 Subject: [PATCH] Added a 'non-abortable' flag to the signout request. Re-enabled cancelling ongoing downloads to allow for a quicker exit of STK. --- src/online/http_request.cpp | 3 ++- src/online/online_player_profile.cpp | 1 + src/online/request.cpp | 1 + src/online/request.hpp | 12 ++++++++++ src/online/request_manager.cpp | 36 +++++++++++++--------------- src/online/request_manager.hpp | 1 - 6 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/online/http_request.cpp b/src/online/http_request.cpp index 5ca7a5658..a29a9384d 100644 --- a/src/online/http_request.cpp +++ b/src/online/http_request.cpp @@ -322,7 +322,8 @@ namespace Online // Check if we are asked to abort the download. If so, signal this // back to libcurl by returning a non-zero status. - if(request_manager->getAbort() || request->isCancelled() ) + if( (request_manager->getAbort() || request->isCancelled()) && + request->isAbortable() ) { // Indicates to abort the current download, which means that this // thread will go back to the mainloop and handle the next request. diff --git a/src/online/online_player_profile.cpp b/src/online/online_player_profile.cpp index 5e7d83b46..ce67302d5 100644 --- a/src/online/online_player_profile.cpp +++ b/src/online/online_player_profile.cpp @@ -240,6 +240,7 @@ namespace Online m_player->setUserDetails(this, UserConfigParams::m_remember_user ? "client-quit" :"disconnect"); + setAbortable(false); } // SignOutRequest }; // SignOutRequest // ---------------------------------------------------------------- diff --git a/src/online/request.cpp b/src/online/request.cpp index 85c1bd798..8fc8672ed 100644 --- a/src/online/request.cpp +++ b/src/online/request.cpp @@ -44,6 +44,7 @@ namespace Online { m_cancel.setAtomic(false); m_state.setAtomic(S_PREPARING); + m_is_abortable.setAtomic(true); } // Request // ------------------------------------------------------------------------ diff --git a/src/online/request.hpp b/src/online/request.hpp index 6250b8cee..743126abd 100644 --- a/src/online/request.hpp +++ b/src/online/request.hpp @@ -105,6 +105,12 @@ namespace Online /** Cancel this request if it is active. */ Synchronised m_cancel; + + /** If this request can be aborted (at the end of STK). Most requests + * can, except the (final) logout and client-quit/signout-request, + * which must be finished even when STK is quitting. */ + Synchronised m_is_abortable; + /** Set to though if the reply of the request is in and callbacks are * executed */ Synchronised m_state; @@ -156,6 +162,12 @@ namespace Online /** Returns if this request is to be canceled. */ bool isCancelled() const { return m_cancel.getAtomic(); } // -------------------------------------------------------------------- + /** Returns if this request can be aborted. */ + bool isAbortable() const { return m_is_abortable.getAtomic(); } + // -------------------------------------------------------------------- + /** Sets if this request is abortable or not. */ + void setAbortable(bool b) { m_is_abortable.setAtomic(b); } + // -------------------------------------------------------------------- /** Sets the request state to busy. */ void setBusy() { diff --git a/src/online/request_manager.cpp b/src/online/request_manager.cpp index 1d6fc2778..4a720a3be 100644 --- a/src/online/request_manager.cpp +++ b/src/online/request_manager.cpp @@ -147,28 +147,26 @@ namespace Online */ void RequestManager::stopNetworkThread() { - // If a download should be active (which means it was cancelled by the - // user, in which case it will still be ongoing in the background) - // we can't get the mutex, and would have to wait for a timeout, - // and we couldn't finish STK. This way we request an abort of - // a download, which mean we can get the mutex and ask the service - // thread here to cancel properly. - //cancelAllDownloads(); FIXME if used this way it also cancels the client-quit action + // This will queue a sign-out or client-quit request PlayerManager::onSTKQuit(); - addRequest(new Request(true, HTTP_MAX_PRIORITY, Request::RT_QUIT)); - } // stopNetworkThread - // ------------------------------------------------------------------------ - /** Signals to the progress function to request any ongoing download to be - * cancelled. This function can also be called if there is actually no - * download atm. The function progressDownload checks m_abort and will - * return a non-zero value which causes libcurl to abort. */ - void RequestManager::cancelAllDownloads() - { + // Put in a high priortity quit request in. It has the same priority + // as a sign-out request (so the sign-out will be executed before the + // quit request). + Request *quit = new Request(true, HTTP_MAX_PRIORITY, Request::RT_QUIT); + quit->setAbortable(false); + addRequest(quit); + + // It is possible that downloads are still ongoing (either an addon + // download that the user aborted, or the addon icons etc are still + // queued). In order to allow a quick exit of stk we set a flag that + // will cause libcurl to abort downloading asap, and then allow the + // other requests (sign-out and quit) to be executed asap. Note that + // the sign-out request is set to be not abortable, so it still will + // be executed (before the quit request is executed, which causes this + // thread to exit). m_abort.setAtomic(true); - // FIXME doesn't get called at the moment. When using this again, - // be sure that HTTP_MAX_PRIORITY requests still get executed. - } // cancelAllDownloads + } // stopNetworkThread // ------------------------------------------------------------------------ /** Inserts a request into the queue of all requests. The request will be diff --git a/src/online/request_manager.hpp b/src/online/request_manager.hpp index 6f060442a..536fe0256 100644 --- a/src/online/request_manager.hpp +++ b/src/online/request_manager.hpp @@ -102,7 +102,6 @@ namespace Online static bool isRunning(); void addRequest(Online::Request *request); - void cancelAllDownloads(); void startNetworkThread(); void stopNetworkThread();