Fix use of previously deallocated EmergeManager

minetest · Jan 26, 2014 · 9b978db · 9b978db
1 parent 6e352e3
commit 9b978db
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 8 deletions.
diff --git 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
@@ -87,6 +87,8 @@ class EmergeManager : public IBackgroundBlockEmerger {
 	std::vector<Mapgen *> mapgen;
 	std::vector<EmergeThread *> emergethread;
 
+	bool threads_active;
+
 	//settings
 	MapgenParams *params;
 	bool mapgen_debug_info;
@@ -119,7 +121,8 @@ class EmergeManager : public IBackgroundBlockEmerger {
 	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
@@ -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 =