From d685ca0619cdbaf61623b673d26a2c9aa8b5b85e Mon Sep 17 00:00:00 2001 From: rubidium Date: Sun, 3 May 2009 15:44:05 +0000 Subject: (svn r16220) -Fix [FS#2862]: possible crashes when quiting OpenTTD or forcing resizes/redraws of the screen during map generation --- src/genworld.cpp | 41 ++++++++++++++--------------------------- src/genworld.h | 8 +++++--- src/genworld_gui.cpp | 18 +++++++++++------- src/gfx.cpp | 20 +++++++++++++------- src/openttd.cpp | 5 +++++ 5 files changed, 48 insertions(+), 44 deletions(-) (limited to 'src') diff --git a/src/genworld.cpp b/src/genworld.cpp index d520292b7..fe5232e8f 100644 --- a/src/genworld.cpp +++ b/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; } diff --git a/src/genworld.h b/src/genworld.h index d46ee731b..522d93f46 100644 --- a/src/genworld.h +++ b/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 */ diff --git a/src/genworld_gui.cpp b/src/genworld_gui.cpp index f9c6082f8..dd5993892 100644 --- a/src/genworld_gui.cpp +++ b/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; } diff --git a/src/gfx.cpp b/src/gfx.cpp index 37f9a3d34..4df12e210 100644 --- a/src/gfx.cpp +++ b/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); - } } /*! diff --git a/src/openttd.cpp b/src/openttd.cpp index 800b791ab..9dac52fe9 100644 --- a/src/openttd.cpp +++ b/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(); -- cgit v1.2.3-70-g09d2