(svn r16220) -Fix [FS#2862]: possible crashes when quiting OpenTTD or forcing resizes/redraws of the screen during map generation

src/genworld.cpp

@@ -47,32 +47,10 @@ void InitializeGame(uint size_x, uint size_y, bool reset_date);
  *  in the genworld.h and genworld.c! -- TrueLight */
 gw_info _gw;
 
-/**
- * Set the status of the Paint flag.
- *  If it is true, the thread will hold with any futher generating till
- *  the drawing of the screen is done. This is handled by
- *  SetGeneratingWorldProgress(), so calling that function will stall
- *  from time to time.
- */
-void SetGeneratingWorldPaintStatus(bool status)
-{
-	_gw.wait_for_draw = status;
-}
-
-/**
- * Returns true if the thread wants the main program to do a (full) paint.
- *  If this returns false, please do not update the screen. Because we are
- *  writing in a thread, it can cause damaged data (reading and writing the
- *  same tile at the same time).
- */
-bool IsGeneratingWorldReadyForPaint()
-{
-	/* If we are in quit_thread mode, ignore this and always return false. This
-	 *  forces the screen to not be drawn, and the GUI not to wait for a draw. */
-	if (!_gw.active || _gw.quit_thread || !_gw.threaded) return false;
-
-	return _gw.wait_for_draw;
-}
+/** Rights for the map generation */
+ThreadMutex *_genworld_mapgen_mutex = ThreadMutex::New();
+/** Rights for the painting */
+ThreadMutex *_genworld_paint_mutex = ThreadMutex::New();
 
 /**
  * Tells if the world generation is done in a thread or not.
@@ -100,6 +78,7 @@ static void CleanupGeneration()
 
 	DeleteWindowById(WC_GENERATE_PROGRESS_WINDOW, 0);
 	MarkWholeScreenDirty();
+	_genworld_mapgen_mutex->EndCritical();
 }
 
 /**
@@ -109,6 +88,7 @@ static void _GenerateWorld(void *arg)
 {
 	try {
 		_generating_world = true;
+		_genworld_mapgen_mutex->BeginCritical();
 		if (_network_dedicated) DEBUG(net, 0, "Generating map, please wait...");
 		/* Set the Random() seed to generation_seed so we produce the same map with the same seed */
 		if (_settings_game.game_creation.generation_seed == GENERATE_NEW_SEED) _settings_game.game_creation.generation_seed = _settings_newgame.game_creation.generation_seed = InteractiveRandom();
@@ -194,6 +174,7 @@ static void _GenerateWorld(void *arg)
 		}
 	} catch (...) {
 		_generating_world = false;
+		_genworld_mapgen_mutex->EndCritical();
 		throw;
 	}
 }
@@ -223,11 +204,16 @@ void GenerateWorldSetAbortCallback(gw_abort_proc *proc)
 void WaitTillGeneratedWorld()
 {
 	if (_gw.thread == NULL) return;
+
+	_genworld_mapgen_mutex->EndCritical();
+	_genworld_paint_mutex->EndCritical();
 	_gw.quit_thread = true;
 	_gw.thread->Join();
 	delete _gw.thread;
 	_gw.thread   = NULL;
 	_gw.threaded = false;
+	_genworld_mapgen_mutex->BeginCritical();
+	_genworld_paint_mutex->BeginCritical();
 }
 
 /**
@@ -280,7 +266,6 @@ void GenerateWorld(GenerateWorldMode mode, uint size_x, uint size_y)
 	_gw.abort  = false;
 	_gw.abortp = NULL;
 	_gw.lc     = _local_company;
-	_gw.wait_for_draw = false;
 	_gw.quit_thread   = false;
 	_gw.threaded      = true;
 
@@ -315,7 +300,9 @@ void GenerateWorld(GenerateWorldMode mode, uint size_x, uint size_y)
 	    !ThreadObject::New(&_GenerateWorld, NULL, &_gw.thread)) {
 		DEBUG(misc, 1, "Cannot create genworld thread, reverting to single-threaded mode");
 		_gw.threaded = false;
+		_genworld_mapgen_mutex->EndCritical();
 		_GenerateWorld(NULL);
+		_genworld_mapgen_mutex->BeginCritical();
 		return;
 	}
 

src/genworld.h

@@ -16,6 +16,8 @@ enum {
 	LG_TERRAGENESIS = 1,  ///< TerraGenesis Perlin landscape generator
 
 	GENERATE_NEW_SEED = UINT_MAX, ///< Create a new random seed
+
+	GENWORLD_REDRAW_TIMEOUT = 200, ///< Timeout between redraws
 };
 
 /* Modes for GenerateWorld */
@@ -32,7 +34,6 @@ typedef void gw_abort_proc();
 struct gw_info {
 	bool active;           ///< Is generating world active
 	bool abort;            ///< Whether to abort the thread ASAP
-	bool wait_for_draw;    ///< Are we waiting on a draw event
 	bool quit_thread;      ///< Do we want to quit the active thread
 	bool threaded;         ///< Whether we run _GenerateWorld threaded
 	GenerateWorldMode mode;///< What mode are we making a world in
@@ -69,8 +70,6 @@ static inline bool IsGeneratingWorld()
 }
 
 /* genworld.cpp */
-void SetGeneratingWorldPaintStatus(bool status);
-bool IsGeneratingWorldReadyForPaint();
 bool IsGenerateWorldThreaded();
 void GenerateWorldSetCallback(gw_done_proc *proc);
 void GenerateWorldSetAbortCallback(gw_abort_proc *proc);
@@ -89,4 +88,7 @@ void StartNewGameWithoutGUI(uint seed);
 void ShowCreateScenario();
 void StartScenarioEditor();
 
+extern class ThreadMutex *_genworld_mapgen_mutex;
+extern class ThreadMutex *_genworld_paint_mutex;
+
 #endif /* GENWORLD_H */

src/genworld_gui.cpp

@@ -26,6 +26,7 @@
 #include "landscape_type.h"
 #include "querystring_gui.h"
 #include "town.h"
+#include "thread.h"
 
 #include "table/strings.h"
 #include "table/sprites.h"
@@ -1352,8 +1353,8 @@ static void _SetGeneratingWorldProgress(gwp_class cls, uint progress, uint total
 		_tp.percent = percent_table[cls];
 	}
 
-	/* Don't update the screen too often. So update it once in every 200ms */
-	if (!_network_dedicated && _tp.timer != 0 && _realtime_tick - _tp.timer < 200) return;
+	/* Don't update the screen too often. So update it once in every once in a while... */
+	if (!_network_dedicated && _tp.timer != 0 && _realtime_tick - _tp.timer < GENWORLD_REDRAW_TIMEOUT) return;
 
 	/* Percentage is about the number of completed tasks, so 'current - 1' */
 	_tp.percent = percent_table[cls] + (percent_table[cls + 1] - percent_table[cls]) * (_tp.current == 0 ? 0 : _tp.current - 1) / _tp.total;
@@ -1379,12 +1380,15 @@ static void _SetGeneratingWorldProgress(gwp_class cls, uint progress, uint total
 
 	InvalidateWindow(WC_GENERATE_PROGRESS_WINDOW, 0);
 	MarkWholeScreenDirty();
-	SetGeneratingWorldPaintStatus(true);
 
-	/* We wait here till the paint is done, so we don't read and write
-	 *  on the same tile at the same moment. Nasty hack, but that happens
-	 *  if you implement threading afterwards */
-	while (IsGeneratingWorldReadyForPaint()) { CSleep(10); }
+	/* Release the rights to the map generator, and acquire the rights to the
+	 * paint thread. The 'other' thread already has the paint thread rights so
+	 * this ensures us that we are waiting until the paint thread is done
+	 * before we reacquire the mapgen rights */
+	_genworld_mapgen_mutex->EndCritical();
+	_genworld_paint_mutex->BeginCritical();
+	_genworld_mapgen_mutex->BeginCritical();
+	_genworld_paint_mutex->EndCritical();
 
 	_tp.timer = _realtime_tick;
 }

src/gfx.cpp

@@ -19,6 +19,7 @@
 #include "landscape_type.h"
 #include "network/network_func.h"
 #include "core/smallvec_type.hpp"
+#include "thread.h"
 
 #include "table/palettes.h"
 #include "table/sprites.h"
@@ -1275,7 +1276,18 @@ void DrawDirtyBlocks()
 	int x;
 	int y;
 
-	if (IsGeneratingWorld() && !IsGeneratingWorldReadyForPaint()) return;
+	if (IsGeneratingWorld()) {
+		/* We are generating the world, so release our rights to the map and
+		 * painting while we are waiting a bit. */
+		_genworld_paint_mutex->EndCritical();
+		_genworld_mapgen_mutex->EndCritical();
+
+		/* Wait a while and update _realtime_tick so we are given the rights */
+		CSleep(GENWORLD_REDRAW_TIMEOUT);
+		_realtime_tick += GENWORLD_REDRAW_TIMEOUT;
+		_genworld_paint_mutex->BeginCritical();
+		_genworld_mapgen_mutex->BeginCritical();
+	}
 
 	y = 0;
 	do {
@@ -1343,12 +1355,6 @@ void DrawDirtyBlocks()
 	_invalid_rect.top = h;
 	_invalid_rect.right = 0;
 	_invalid_rect.bottom = 0;
-
-	/* If we are generating a world, and waiting for a paint run, mark it here
-	 *  as done painting, so we can continue generating. */
-	if (IsGeneratingWorld() && IsGeneratingWorldReadyForPaint()) {
-		SetGeneratingWorldPaintStatus(false);
-	}
 }
 
 /*!

src/openttd.cpp

@@ -50,6 +50,7 @@
 #include "elrail_func.h"
 #include "rev.h"
 #include "highscore.h"
+#include "thread.h"
 
 #include "newgrf_commons.h"
 
@@ -655,6 +656,10 @@ int ttd_main(int argc, char *argv[])
 	InitializeGUI();
 	IConsoleCmdExec("exec scripts/autoexec.scr 0");
 
+	/* Take our initial lock on whatever we might want to do! */
+	_genworld_paint_mutex->BeginCritical();
+	_genworld_mapgen_mutex->BeginCritical();
+
 	GenerateWorld(GW_EMPTY, 64, 64); // Make the viewport initialization happy
 	WaitTillGeneratedWorld();