From 9b978db0c2578b06c5669096e325c1ce70864edc Mon Sep 17 00:00:00 2001 From: kwolekr Date: Sun, 26 Jan 2014 01:12:18 -0500 Subject: [PATCH] Fix use of previously deallocated EmergeManager --- src/emerge.cpp | 38 ++++++++++++++++++++++++++++++++++---- src/emerge.h | 5 ++++- src/server.cpp | 11 ++++++++--- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/emerge.cpp b/src/emerge.cpp index ff00a0b62..bd9b7c7bd 100644 --- a/src/emerge.cpp +++ b/src/emerge.cpp @@ -92,6 +92,11 @@ EmergeManager::EmergeManager(IGameDef *gamedef) { this->biomedef = new BiomeDefManager(); this->params = NULL; + // Note that accesses to this variable are not synchronized. + // This is because the *only* thread ever starting or stopping + // EmergeThreads should be the ServerThread. + this->threads_active = false; + this->luaoverride_params = NULL; this->luaoverride_params_modified = 0; this->luaoverride_flagmask = 0; @@ -128,9 +133,11 @@ EmergeManager::EmergeManager(IGameDef *gamedef) { EmergeManager::~EmergeManager() { for (unsigned int i = 0; i != emergethread.size(); i++) { - emergethread[i]->Stop(); - emergethread[i]->qevent.signal(); - emergethread[i]->Wait(); + if (threads_active) { + emergethread[i]->Stop(); + emergethread[i]->qevent.signal(); + emergethread[i]->Wait(); + } delete emergethread[i]; delete mapgen[i]; } @@ -252,9 +259,32 @@ Mapgen *EmergeManager::getCurrentMapgen() { } -void EmergeManager::startAllThreads() { +void EmergeManager::startThreads() { + if (threads_active) + return; + for (unsigned int i = 0; i != emergethread.size(); i++) emergethread[i]->Start(); + + threads_active = true; +} + + +void EmergeManager::stopThreads() { + if (!threads_active) + return; + + // Request thread stop in parallel + for (unsigned int i = 0; i != emergethread.size(); i++) { + emergethread[i]->Stop(); + emergethread[i]->qevent.signal(); + } + + // Then do the waiting for each + for (unsigned int i = 0; i != emergethread.size(); i++) + emergethread[i]->Wait(); + + threads_active = false; } diff --git a/src/emerge.h b/src/emerge.h index b2b00adc9..17097327b 100644 --- a/src/emerge.h +++ b/src/emerge.h @@ -87,6 +87,8 @@ public: std::vector mapgen; std::vector emergethread; + bool threads_active; + //settings MapgenParams *params; bool mapgen_debug_info; @@ -119,7 +121,8 @@ public: Mapgen *createMapgen(std::string mgname, int mgid, MapgenParams *mgparams); MapgenParams *createMapgenParams(std::string mgname); - void startAllThreads(); + void startThreads(); + void stopThreads(); bool enqueueBlockEmerge(u16 peer_id, v3s16 p, bool allow_generate); void registerMapgen(std::string name, MapgenFactory *mgfactory); diff --git a/src/server.cpp b/src/server.cpp index 47b11d3da..0ada4d818 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -917,8 +917,9 @@ Server::~Server() stop(); delete m_thread; - //shutdown all emerge threads first! - delete m_emerge; + // stop all emerge threads before deleting players that may have + // requested blocks to be emerged + m_emerge->stopThreads(); /* Delete clients @@ -938,6 +939,10 @@ Server::~Server() // Delete things in the reverse order of creation delete m_env; + + // N.B. the EmergeManager should be deleted after the Environment since Map + // depends on EmergeManager to write its current params to the map meta + delete m_emerge; delete m_rollback; delete m_banmanager; delete m_event; @@ -1684,7 +1689,7 @@ void Server::AsyncRunStep(bool initial_step) { counter = 0.0; - m_emerge->startAllThreads(); + m_emerge->startThreads(); // Update m_enable_rollback_recording here too m_enable_rollback_recording =