From c62cc7fbaad31a6cce18d26fdd0e4943fea51cad Mon Sep 17 00:00:00 2001 From: hiker Date: Wed, 4 Jun 2014 16:51:29 +1000 Subject: [PATCH] Added CanBeDeleted class, which implements a timeout function. It is used to make sure that (in this case) the NewsManager thread does not need the file manager anymore when stk exists (which can only happen on very very quick exists, and slow downloads). This avoids a potential crash that the NewsManager thread could access the file manager after the file manager was deleted. --- src/addons/news_manager.cpp | 13 ++++--- src/addons/news_manager.hpp | 3 +- src/graphics/irr_driver.cpp | 9 ++--- src/main.cpp | 39 ++++++++++++--------- src/utils/can_be_deleted.hpp | 68 ++++++++++++++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 25 deletions(-) create mode 100755 src/utils/can_be_deleted.hpp diff --git a/src/addons/news_manager.cpp b/src/addons/news_manager.cpp index 99498ef11..4c6d964c4 100644 --- a/src/addons/news_manager.cpp +++ b/src/addons/news_manager.cpp @@ -49,9 +49,10 @@ NewsManager::~NewsManager() // --------------------------------------------------------------------------- /** This function initialises the data for the news manager. It starts a - * separate thread to execute downloadNews() - which (if necessary) the - * news.xml file and updating the list of news messages. It also initialises - * the addons manager (which can trigger another download of news.xml). + * separate thread to execute downloadNews() - which (if necessary) downaloads + * the news.xml file and updates the list of news messages. It also + * initialises the addons manager (which can trigger another download of + * news.xml). * \param force_refresh Re-download news.xml, even if */ void NewsManager::init(bool force_refresh) @@ -183,7 +184,7 @@ void* NewsManager::downloadNews(void *obj) if(xml) delete xml; xml = NULL; - + // Process new.xml now. if(file_manager->fileExists(xml_file)) { @@ -194,6 +195,10 @@ void* NewsManager::downloadNews(void *obj) delete xml; } + // We can't finish stk (esp. delete the file manager) before + // this part of the code is reached (since otherwise the file + // manager might be access after it was deleted). + me->setCanBeDeleted(); pthread_exit(NULL); return 0; // prevent warning } // downloadNews diff --git a/src/addons/news_manager.hpp b/src/addons/news_manager.hpp index 9e76f80fd..264d7936a 100644 --- a/src/addons/news_manager.hpp +++ b/src/addons/news_manager.hpp @@ -25,6 +25,7 @@ #include using namespace irr; +#include "utils/can_be_deleted.hpp" #include "utils/synchronised.hpp" class XMLNode; @@ -32,7 +33,7 @@ class XMLNode; /** * \ingroup addonsgroup */ -class NewsManager +class NewsManager : public CanBeDeleted { private: static NewsManager *m_news_manager; diff --git a/src/graphics/irr_driver.cpp b/src/graphics/irr_driver.cpp index c9e200143..a45c84892 100644 --- a/src/graphics/irr_driver.cpp +++ b/src/graphics/irr_driver.cpp @@ -211,7 +211,10 @@ Window get_toplevel_parent(Display* display, Window window) #endif // ---------------------------------------------------------------------------- - +/** If the position of the window should be remembered, store it in the config + * file. + * \post The user config file must still be saved! + */ void IrrDriver::updateConfigIfRelevant() { if (!UserConfigParams::m_fullscreen && @@ -241,7 +244,6 @@ void IrrDriver::updateConfigIfRelevant() { UserConfigParams::m_window_x = x; UserConfigParams::m_window_y = y; - user_config->saveConfig(); } } else @@ -265,11 +267,10 @@ void IrrDriver::updateConfigIfRelevant() { UserConfigParams::m_window_x = wx; UserConfigParams::m_window_y = wy; - user_config->saveConfig(); } #endif } -} +} // updateConfigIfRelevant // ---------------------------------------------------------------------------- /** Gets a list of supported video modes from the irrlicht device. This data diff --git a/src/main.cpp b/src/main.cpp index b7a4ab04c..de27b3a91 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1375,13 +1375,6 @@ int main(int argc, char *argv[] ) /* Program closing...*/ - if(user_config) - { - // In case that abort is triggered before user_config exists - if (UserConfigParams::m_crashed) UserConfigParams::m_crashed = false; - user_config->saveConfig(); - } - #ifdef ENABLE_WIIUSE if(wiimote_manager) delete wiimote_manager; @@ -1433,11 +1426,16 @@ static void cleanSuperTuxKart() delete main_loop; - irr_driver->updateConfigIfRelevant(); - if(Online::RequestManager::isRunning()) Online::RequestManager::get()->stopNetworkThread(); + 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(); @@ -1445,12 +1443,6 @@ static void cleanSuperTuxKart() Online::ProfileManager::destroy(); GUIEngine::DialogQueue::deallocate(); - AchievementsManager::destroy(); - Referee::cleanup(); - - if(ReplayPlay::get()) ReplayPlay::destroy(); - if(race_manager) delete race_manager; - NewsManager::deallocate(); if(addons_manager) delete addons_manager; NetworkManager::kill(); @@ -1471,6 +1463,14 @@ static void cleanSuperTuxKart() PlayerManager::destroy(); if(unlock_manager) delete unlock_manager; + // 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."); + } + NewsManager::deallocate(); cleanUserConfig(); StateManager::deallocate(); @@ -1485,7 +1485,14 @@ static void cleanUserConfig() { if(stk_config) delete stk_config; if(translations) delete translations; - if(user_config) delete user_config; + if (user_config) + { + // In case that abort is triggered before user_config exists + if (UserConfigParams::m_crashed) UserConfigParams::m_crashed = false; + user_config->saveConfig(); + delete user_config; + } + if(file_manager) delete file_manager; if(irr_driver) delete irr_driver; } diff --git a/src/utils/can_be_deleted.hpp b/src/utils/can_be_deleted.hpp new file mode 100755 index 000000000..9bf0879b4 --- /dev/null +++ b/src/utils/can_be_deleted.hpp @@ -0,0 +1,68 @@ +// +// SuperTuxKart - a fun racing game with go-kart +// Copyright (C) 2010-2013 Joerg Henrichs +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 3 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + +#ifndef HEADER_CAN_BE_DELETED +#define HEADER_CAN_BE_DELETED + +#include "utils/log.hpp" +#include "utils/synchronised.hpp" +#include "utils/time.hpp" + +/** A simple class that a +*/ +class CanBeDeleted +{ +private: + Synchronised m_can_be_deleted; +public: + /** Set this instance to be not ready to be deleted. */ + CanBeDeleted() { m_can_be_deleted.setAtomic(false); } + // ------------------------------------------------------------------------ + /** Sets this instance to be ready to be deleted. */ + void setCanBeDeleted() {m_can_be_deleted.setAtomic(true); } + // ------------------------------------------------------------------------ + /** Waits at most t seconds for this class to be ready to be deleted. + * \return true if the class is ready, false in case of a time out. + */ + bool waitForReadyToDeleted(float waiting_time) + { + if (m_can_be_deleted.getAtomic()) return true; + double start = StkTime::getRealTime(); + while(1) + { + if(m_can_be_deleted.getAtomic()) + { + Log::verbose("Thread", + "Waited %lf for thread to become deleteable.", + StkTime::getRealTime()-start); + return true; + } + StkTime::sleep(10); + if(StkTime::getRealTime() - start > waiting_time) + { + Log::verbose("Thread", "Waited for more than %f seconds for " + "thread to become deleteable", + waiting_time); + return false; + } + } // while 1 + } // waitForReadyToDeleted + + // ------------------------------------------------------------------------ +}; // CanBeDeleted +#endif \ No newline at end of file