diff --git a/sources.cmake b/sources.cmake index 9f7e43df6..fd74b86e3 100644 --- a/sources.cmake +++ b/sources.cmake @@ -1,3 +1,5 @@ +# Modify this file to change the last-modified date when you add/remove a file. +# This will then trigger a new cmake run automatically. file(GLOB_RECURSE STK_SOURCES RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "src/*.cpp") file(GLOB_RECURSE STK_HEADERS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "src/*.hpp") -file(GLOB_RECURSE STK_SHADERS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "data/shaders/*") \ No newline at end of file +file(GLOB_RECURSE STK_SHADERS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "data/shaders/*") diff --git a/src/main.cpp b/src/main.cpp index de27b3a91..775357c65 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1432,20 +1432,9 @@ static void cleanSuperTuxKart() irr_driver->updateConfigIfRelevant(); AchievementsManager::destroy(); Referee::cleanup(); - if(ReplayPlay::get()) ReplayPlay::destroy(); - if(race_manager) delete race_manager; - - - //delete in reverse order of what they were created in. - //see InitTuxkart() - Online::RequestManager::deallocate(); - Online::ServersManager::deallocate(); - Online::ProfileManager::destroy(); - GUIEngine::DialogQueue::deallocate(); - + if(ReplayPlay::get()) ReplayPlay::destroy(); + if(race_manager) delete race_manager; if(addons_manager) delete addons_manager; - NetworkManager::kill(); - if(grand_prix_manager) delete grand_prix_manager; if(highscore_manager) delete highscore_manager; if(attachment_manager) delete attachment_manager; @@ -1462,15 +1451,34 @@ static void cleanSuperTuxKart() delete ParticleKindManager::get(); PlayerManager::destroy(); if(unlock_manager) delete unlock_manager; + Online::ProfileManager::destroy(); + GUIEngine::DialogQueue::deallocate(); + + // Now finish shutting down objects which a separate thread. The + // RequestManager has been signaled to shut down as early as possible, + // the NewsManager thread should have finished quite early on anyway. + // But still give them some additional time to finish. It avoids a + // race condition where a thread might access the file manager after it + // was deleted (in cleanUserConfig below), but before STK finishes and + // the os takes all threads down. - // Wait (up to 2 seconds) for the news manager to be ready to be deleted, - // i.e. make sure it does not need the file_manager anymore (which will - // get deleted in cleanUserConfig). if(!NewsManager::get()->waitForReadyToDeleted(2.0f)) { - Log::info("NewsManager", "News manager not stopping, exiting anyway."); + Log::info("Thread", "News manager not stopping, exiting anyway."); } NewsManager::deallocate(); + + if(!Online::RequestManager::get()->waitForReadyToDeleted(5.0f)) + { + Log::info("Thread", "Request Manager not aborting in time, aborting."); + } + Online::RequestManager::deallocate(); + + // FIXME: do we need to wait for threads there, can they be + // moved further up? + Online::ServersManager::deallocate(); + NetworkManager::kill(); + cleanUserConfig(); StateManager::deallocate(); diff --git a/src/online/request_manager.cpp b/src/online/request_manager.cpp index 4a720a3be..f9ed84cb4 100644 --- a/src/online/request_manager.cpp +++ b/src/online/request_manager.cpp @@ -1,7 +1,7 @@ // // SuperTuxKart - a fun racing game with go-kart // Copyright (C) 2010-2014 Lucas Baudin -// 2011-201 Joerg Henrichs +// 2011-2014 Joerg Henrichs // 2013-2014 Glenn De Jonghe // // This program is free software; you can redistribute it and/or @@ -83,7 +83,7 @@ namespace Online pthread_cond_init(&m_cond_request, NULL); m_abort.setAtomic(false); m_time_since_poll = MENU_POLLING_INTERVAL * 0.9; - } + } // RequestManager // ------------------------------------------------------------------------ RequestManager::~RequestManager() @@ -94,8 +94,7 @@ namespace Online m_thread_id.unlock(); pthread_cond_destroy(&m_cond_request); curl_global_cleanup(); - } - + } // ~RequestManager // ------------------------------------------------------------------------ /** Start the actual network thread. This can not be done as part of @@ -220,6 +219,11 @@ namespace Online me->m_request_queue.lock(); } // while + // Signal that the request manager can now be deleted. + // We signal this even before cleaning up memory, since there's no + // need to keep the user waiting for STK to exit. + me->setCanBeDeleted(); + // At this stage we have the lock for m_request_queue while(!me->m_request_queue.getData().empty()) { diff --git a/src/online/request_manager.hpp b/src/online/request_manager.hpp index 536fe0256..91005e186 100644 --- a/src/online/request_manager.hpp +++ b/src/online/request_manager.hpp @@ -23,6 +23,7 @@ #include "io/xml_node.hpp" #include "online/request.hpp" +#include "utils/can_be_deleted.hpp" #include "utils/string_utils.hpp" #include "utils/synchronised.hpp" @@ -42,11 +43,41 @@ namespace Online { - /** - * \brief Class to connect with a server over HTTP(S) - * \ingroup online - */ - class RequestManager + /** A class to execute requests in a separate thread. Typically the + * requests involve a http(s) requests to be sent to the stk server, and + * receive an answer (e.g. to sign in; or to download an addon). The + * requests are sorted by priority (e.g. sign in and out have higher + * priority than downloading addon icons). + * A request is created and initialised from the main thread. When it + * is moved into the request queue, it must not be handled by the main + * thread anymore, only the RequestManager thread can handle it. + * Once the request is finished, it is put in a separate ready queue. + * The main thread regularly checks the ready queue for any ready + * request, and executes a callback. So there is no need to protect + * any functions or data members in requests, since they will either + * be handled by the main thread, or RequestManager thread, never by + * both. + * On exit, if necessary a high priority sign-out or client-quit request + * is put into the queue, and a flag is set which causes libcurl to + * abort any ongoing download. Then an additional 'quit' event with + * same priority as the sign-out is added to the queue (since it will + * be added later, the sign-out will be executed first, making sure that + * a logged in user is logged out (or its session saved). Once this is + * done, most of stk is deleted (except a few objects like the file + * manager which might be accessed if a download just finished before the + * abort). On executing the quit request, the request manager will set + * a flag that it is ready to be deleted (using the CanBeDeleted class). + * The main thread will wait for a certain amount of time for the + * RequestManager to be ready to be deleted (i.e. the sign-out and quit + * request have been processes), before deleting the RequestManager. + * Typically the RequestManager will finish while the rest of stk is + * shutting down, so the user will not experience any waiting time. Only + * on first start of stk (which will trigger downloading of all addon + * icons) is it possible that actually a download request is running, + * which might take a bit before it can be deleted. + * \ingroup online + */ + class RequestManager : public CanBeDeleted { public: /** If stk has permission to access the internet (for news diff --git a/src/utils/can_be_deleted.hpp b/src/utils/can_be_deleted.hpp index 9bf0879b4..2774b1b74 100755 --- a/src/utils/can_be_deleted.hpp +++ b/src/utils/can_be_deleted.hpp @@ -23,8 +23,16 @@ #include "utils/synchronised.hpp" #include "utils/time.hpp" -/** A simple class that a -*/ +/** A simple class that a adds a function to wait with a timeout for a + * class to be ready to be deleted. It is used for objects with their + * own threads (e.g. RequestManager) to make sure they can be deleted. + * For example, the RequestManager might be executing a download request. + * So we have to signal libcurl to abort the download request, then + * potentially handle a high priority sign-out request before the thread + * can be deleted. With this object the main thread can wait for a given + * amount of time (in case that of a bad internet hickup) before deleting + * the RequestManager. + */ class CanBeDeleted { private: @@ -43,18 +51,21 @@ public: { if (m_can_be_deleted.getAtomic()) return true; double start = StkTime::getRealTime(); + Log::verbose("Thread", "Start waiting %lf", start); while(1) { if(m_can_be_deleted.getAtomic()) { Log::verbose("Thread", - "Waited %lf for thread to become deleteable.", - StkTime::getRealTime()-start); + "Waited %lf seconds for thread to become deleteable.", + StkTime::getRealTime()-start); + Log::verbose("Thread", "Stop waiting %lf", StkTime::getRealTime()); return true; } StkTime::sleep(10); if(StkTime::getRealTime() - start > waiting_time) { + Log::verbose("Thread", "Stop waiting %lf", StkTime::getRealTime()); Log::verbose("Thread", "Waited for more than %f seconds for " "thread to become deleteable", waiting_time);