Windows: Avoid destruction of unjoined std::thread on ExitProcess

See: #649

src/music/dmusic.cpp

@@ -121,7 +121,7 @@ struct PlaybackSegment {
 	bool loop;
 };
 
-static struct {
+struct DMusicPlayback {
 	bool shutdown;    ///< flag to indicate playback thread shutdown
 	bool playing;     ///< flag indicating that playback is active
 	bool do_start;    ///< flag for starting playback of next_file at next opportunity
@@ -132,14 +132,29 @@ static struct {
 
 	MidiFile next_file;           ///< upcoming file to play
 	PlaybackSegment next_segment; ///< segment info for upcoming file
-} _playback;
 
-/** Handle to our worker thread. */
+	/** Handle to our worker thread. */
-static std::thread _dmusic_thread;
+	std::thread dmusic_thread;
-/** Event to signal the thread that it should look at a state change. */
+	/** Event to signal the thread that it should look at a state change. */
-static HANDLE _thread_event = nullptr;
+	HANDLE thread_event = nullptr;
-/** Lock access to playback data that is not thread-safe. */
+	/** Lock access to playback data that is not thread-safe. */
-static std::mutex _thread_mutex;
+	std::mutex thread_mutex;
+
+	void StopThread()
+	{
+		if (this->dmusic_thread.joinable()) {
+			this->shutdown = true;
+			SetEvent(this->thread_event);
+			this->dmusic_thread.join();
+		}
+	}
+
+	~DMusicPlayback()
+	{
+		this->StopThread();
+	}
+};
+static DMusicPlayback _playback;
 
 /** The direct music object manages buffers and ports. */
 static IDirectMusic *_music = nullptr;
@@ -610,7 +625,7 @@ static void MidiThreadProc()
 	DWORD next_timeout = 1000;
 	while (true) {
 		/* Wait for a signal from the GUI thread or until the time for the next event has come. */
-		DWORD wfso = WaitForSingleObject(_thread_event, next_timeout);
+		DWORD wfso = WaitForSingleObject(_playback.thread_event, next_timeout);
 
 		if (_playback.shutdown) {
 			_playback.playing = false;
@@ -636,7 +651,7 @@ static void MidiThreadProc()
 				DEBUG(driver, 2, "DMusic thread: Starting playback");
 				{
 					/* New scope to limit the time the mutex is locked. */
-					std::lock_guard<std::mutex> lock(_thread_mutex);
+					std::lock_guard<std::mutex> lock(_playback.thread_mutex);
 
 					current_file.MoveFrom(_playback.next_file);
 					std::swap(_playback.next_segment, current_segment);
@@ -1148,10 +1163,10 @@ const char *MusicDriver_DMusic::Start(const StringList &parm)
 	if (dls != nullptr) return dls;
 
 	/* Create playback thread and synchronization primitives. */
-	_thread_event = CreateEvent(nullptr, FALSE, FALSE, nullptr);
+	_playback.thread_event = CreateEvent(nullptr, FALSE, FALSE, nullptr);
-	if (_thread_event == nullptr) return "Can't create thread shutdown event";
+	if (_playback.thread_event == nullptr) return "Can't create thread shutdown event";
 
-	if (!StartNewThread(&_dmusic_thread, "ottd:dmusic", &MidiThreadProc)) return "Can't create MIDI output thread";
+	if (!StartNewThread(&_playback.dmusic_thread, "ottd:dmusic", &MidiThreadProc)) return "Can't create MIDI output thread";
 
 	return nullptr;
 }
@@ -1165,11 +1180,7 @@ MusicDriver_DMusic::~MusicDriver_DMusic()
 
 void MusicDriver_DMusic::Stop()
 {
-	if (_dmusic_thread.joinable()) {
+	_playback.StopThread();
-		_playback.shutdown = true;
-		SetEvent(_thread_event);
-		_dmusic_thread.join();
-	}
 
 	/* Unloaded any instruments we loaded. */
 	if (!_dls_downloads.empty()) {
@@ -1203,7 +1214,7 @@ void MusicDriver_DMusic::Stop()
 		_music = nullptr;
 	}
 
-	CloseHandle(_thread_event);
+	CloseHandle(_playback.thread_event);
 
 	CoUninitialize();
 }
@@ -1211,7 +1222,7 @@ void MusicDriver_DMusic::Stop()
 
 void MusicDriver_DMusic::PlaySong(const MusicSongInfo &song)
 {
-	std::lock_guard<std::mutex> lock(_thread_mutex);
+	std::lock_guard<std::mutex> lock(_playback.thread_mutex);
 
 	if (!_playback.next_file.LoadSong(song)) return;
 
@@ -1220,14 +1231,14 @@ void MusicDriver_DMusic::PlaySong(const MusicSongInfo &song)
 	_playback.next_segment.loop = song.loop;
 
 	_playback.do_start = true;
-	SetEvent(_thread_event);
+	SetEvent(_playback.thread_event);
 }
 
 
 void MusicDriver_DMusic::StopSong()
 {
 	_playback.do_stop = true;
-	SetEvent(_thread_event);
+	SetEvent(_playback.thread_event);
 }
 
 

src/sl/saveload.cpp

@@ -479,8 +479,56 @@ void NORETURN CDECL SlErrorCorruptFmt(const char *format, ...)
 }
 
 typedef void (*AsyncSaveFinishProc)();                      ///< Callback for when the savegame loading is finished.
-static std::atomic<AsyncSaveFinishProc> _async_save_finish; ///< Callback to call when the savegame loading is finished.
+
-static std::thread _save_thread;                            ///< The thread we're using to compress and write a savegame
+struct AsyncSaveThread {
+	std::atomic<bool> exit_thread;                ///< Signal that the thread should exit early
+	std::atomic<AsyncSaveFinishProc> finish_proc; ///< Callback to call when the savegame saving is finished.
+	std::thread save_thread;                      ///< The thread we're using to compress and write a savegame
+
+	void SetAsyncSaveFinish(AsyncSaveFinishProc proc)
+	{
+		if (_exit_game || this->exit_thread.load(std::memory_order_relaxed)) return;
+
+		while (this->finish_proc.load(std::memory_order_acquire) != nullptr) {
+			CSleep(10);
+			if (_exit_game || this->exit_thread.load(std::memory_order_relaxed)) return;
+		}
+
+		this->finish_proc.store(proc, std::memory_order_release);
+	}
+
+	void ProcessAsyncSaveFinish()
+	{
+		AsyncSaveFinishProc proc = this->finish_proc.exchange(nullptr, std::memory_order_acq_rel);
+		if (proc == nullptr) return;
+
+		proc();
+
+		if (this->save_thread.joinable()) {
+			this->save_thread.join();
+		}
+	}
+
+	void WaitTillSaved()
+	{
+		if (!this->save_thread.joinable()) return;
+
+		this->save_thread.join();
+
+		/* Make sure every other state is handled properly as well. */
+		this->ProcessAsyncSaveFinish();
+	}
+
+	~AsyncSaveThread()
+	{
+		this->exit_thread.store(true, std::memory_order_relaxed);
+
+		if (this->save_thread.joinable()) {
+			this->save_thread.join();
+		}
+	}
+};
+static AsyncSaveThread _async_save_thread;
 
 /**
  * Called by save thread to tell we finished saving.
@@ -488,10 +536,7 @@ static std::thread _save_thread;                            ///< The thread we'r
  */
 static void SetAsyncSaveFinish(AsyncSaveFinishProc proc)
 {
-	if (_exit_game) return;
+	_async_save_thread.SetAsyncSaveFinish(proc);
-	while (_async_save_finish.load(std::memory_order_acquire) != nullptr) CSleep(10);
-
-	_async_save_finish.store(proc, std::memory_order_release);
 }
 
 /**
@@ -499,14 +544,7 @@ static void SetAsyncSaveFinish(AsyncSaveFinishProc proc)
  */
 void ProcessAsyncSaveFinish()
 {
-	AsyncSaveFinishProc proc = _async_save_finish.exchange(nullptr, std::memory_order_acq_rel);
+	_async_save_thread.ProcessAsyncSaveFinish();
-	if (proc == nullptr) return;
-
-	proc();
-
-	if (_save_thread.joinable()) {
-		_save_thread.join();
-	}
 }
 
 /**
@@ -3492,12 +3530,7 @@ static SaveOrLoadResult SaveFileToDisk(bool threaded)
 
 void WaitTillSaved()
 {
-	if (!_save_thread.joinable()) return;
+	_async_save_thread.WaitTillSaved();
-
-	_save_thread.join();
-
-	/* Make sure every other state is handled properly as well. */
-	ProcessAsyncSaveFinish();
 }
 
 /**
@@ -3523,7 +3556,7 @@ static SaveOrLoadResult DoSave(SaveFilter *writer, bool threaded)
 
 	SaveFileStart();
 
-	if (!threaded || !StartNewThread(&_save_thread, "ottd:savegame", &SaveFileToDisk, true)) {
+	if (!threaded || !StartNewThread(&_async_save_thread.save_thread, "ottd:savegame", &SaveFileToDisk, true)) {
 		if (threaded) DEBUG(sl, 1, "Cannot create savegame thread, reverting to single-threaded mode...");
 
 		SaveOrLoadResult result = SaveFileToDisk(false);