Crashlog: Update thread/lock handling to match GameLoop changes

src/crashlog.cpp

@@ -78,7 +78,6 @@
 /* static */ const char *CrashLog::message = nullptr;
 /* static */ char *CrashLog::gamelog_buffer = nullptr;
 /* static */ const char *CrashLog::gamelog_last = nullptr;
-/* static */ const CrashLog *CrashLog::main_thread_pending_crashlog = nullptr;
 
 char *CrashLog::LogCompiler(char *buffer, const char *last) const
 {
@@ -441,7 +440,7 @@ char *CrashLog::FillCrashLog(char *buffer, const char *last) const
 	buffer = this->LogError(buffer, last, CrashLog::message);
 
 #ifdef USE_SCOPE_INFO
-	if (IsMainThread()) {
+	if (IsMainThread() || IsGameThread()) {
 		buffer += WriteScopeLog(buffer, last);
 	}
 #endif
@@ -644,6 +643,10 @@ bool CrashLog::MakeCrashLog()
 	if (crashlogged) return false;
 	crashlogged = true;
 
+	if (!VideoDriver::EmergencyAcquireGameLock(20, 2)) {
+		printf("Failed to acquire gamelock before filling crash log\n\n");
+	}
+
 	char filename[MAX_PATH];
 	char buffer[65536 * 4];
 	bool ret = true;
@@ -679,15 +682,10 @@ bool CrashLog::MakeCrashLog()
 	_savegame_DBGL_data = buffer;
 	_save_DBGC_data = true;
 
-	if (IsNonMainThread()) {
+	if (!VideoDriver::EmergencyAcquireGameLock(1000, 5)) {
-		printf("Asking main thread to write crash savegame and screenshot...\n\n");
+		printf("Failed to acquire gamelock before writing crash savegame and screenshot, proceeding without lock as current owner is probably stuck\n\n");
-		CrashLog::main_thread_pending_crashlog = this;
-		_exit_game = true;
-		CSleep(60000);
-		if (!CrashLog::main_thread_pending_crashlog) return ret;
-		printf("Main thread did not write crash savegame and screenshot within 60s, trying it from this thread...\n\n");
 	}
-	CrashLog::main_thread_pending_crashlog = nullptr;
+
 	bret = CrashLog::MakeCrashSavegameAndScreenshot();
 	if (!bret) ret = false;
 
@@ -802,18 +800,6 @@ bool CrashLog::MakeCrashSavegameAndScreenshot() const
 	return ret;
 }
 
-/* static */ void CrashLog::MainThreadExitCheckPendingCrashlog()
-{
-	const CrashLog *cl = CrashLog::main_thread_pending_crashlog;
-	if (cl) {
-		CrashLog::main_thread_pending_crashlog = nullptr;
-		cl->MakeCrashSavegameAndScreenshot();
-
-		CrashLog::AfterCrashLogCleanup();
-		abort();
-	}
-}
-
 /**
  * Sets a message for the error message handler.
  * @param message The error message of the error.

src/crashlog.h

@@ -173,10 +173,6 @@ public:
 	inline const char *GetMessage() const { return this->message; }
 
 	static const char *GetAbortCrashlogReason();
-
-	static const CrashLog *main_thread_pending_crashlog;
-
-	static void MainThreadExitCheckPendingCrashlog();
 };
 
 #endif /* CRASHLOG_H */

src/openttd.cpp

@@ -997,8 +997,6 @@ int openttd_main(int argc, char *argv[])
 
 	VideoDriver::GetInstance()->MainLoop();
 
-	CrashLog::MainThreadExitCheckPendingCrashlog();
-
 	WaitTillSaved();
 
 	/* only save config if we have to */

src/os/os2/os2.cpp

@@ -219,8 +219,10 @@ void SetCurrentThreadName(const char *)
 int GetCurrentThreadName(char *str, const char *last) { return 0; }
 
 void SetSelfAsMainThread() { }
+void SetSelfAsGameThread() { }
 void PerThreadSetup() { }
 void PerThreadSetupInit() { }
 
 bool IsMainThread() { return false; }
 bool IsNonMainThread() { return false; }
+bool IsGameThread() { return false; }

src/os/unix/unix.cpp

@@ -341,6 +341,7 @@ int GetCurrentThreadName(char *str, const char *last)
 }
 
 static pthread_t main_thread;
+static pthread_t game_thread;
 
 void SetSelfAsMainThread()
 {
@@ -349,6 +350,13 @@ void SetSelfAsMainThread()
 #endif
 }
 
+void SetSelfAsGameThread()
+{
+#if !defined(NO_THREADS)
+	game_thread = pthread_self();
+#endif
+}
+
 void PerThreadSetup() { }
 
 void PerThreadSetupInit() { }
@@ -370,3 +378,12 @@ bool IsNonMainThread()
 	return false;
 #endif
 }
+
+bool IsGameThread()
+{
+#if !defined(NO_THREADS)
+	return game_thread == pthread_self();
+#else
+	return false;
+#endif
+}

src/os/windows/crashlog_win.cpp

@@ -639,6 +639,8 @@ static LONG WINAPI ExceptionHandler(EXCEPTION_POINTERS *ep)
 		ExitProcess(3);
 	}
 
+	VideoDriver::EmergencyAcquireGameLock(1000, 5);
+
 	CrashLogWindows *log = new CrashLogWindows(ep);
 	CrashLogWindows::current = log;
 	char *buf = log->FillCrashLog(log->crashlog, lastof(log->crashlog));

src/os/windows/win32.cpp

@@ -734,12 +734,18 @@ int OTTDStringCompare(const char *s1, const char *s2)
 }
 
 static DWORD main_thread_id;
+static DWORD game_thread_id;
 
 void SetSelfAsMainThread()
 {
 	main_thread_id = GetCurrentThreadId();
 }
 
+void SetSelfAsGameThread()
+{
+	game_thread_id = GetCurrentThreadId();
+}
+
 static BOOL (WINAPI *_SetThreadStackGuarantee)(PULONG) = nullptr;
 
 void PerThreadSetup()
@@ -765,6 +771,11 @@ bool IsNonMainThread()
 	return main_thread_id != GetCurrentThreadId();
 }
 
+bool IsGameThread()
+{
+	return game_thread_id == GetCurrentThreadId();
+}
+
 static std::map<DWORD, std::string> _thread_name_map;
 static std::mutex _thread_name_map_mutex;
 

src/thread.h

@@ -46,6 +46,11 @@ int GetCurrentThreadName(char *str, const char *last);
  */
 void SetSelfAsMainThread();
 
+/**
+ * Set the current thread as the "game" thread
+ */
+void SetSelfAsGameThread();
+
 /**
  * Perform per-thread setup
  */
@@ -57,15 +62,20 @@ void PerThreadSetup();
 void PerThreadSetupInit();
 
 /**
- * @return true if the current thread definitely the "main" thread. If in doubt returns false.
+ * @return true if the current thread is definitely the "main" thread. If in doubt returns false.
  */
 bool IsMainThread();
 
 /**
- * @return true if the current thread definitely a "non-main" thread. If in doubt returns false.
+ * @return true if the current thread is definitely a "non-main" thread. If in doubt returns false.
  */
 bool IsNonMainThread();
 
+/**
+ * @return true if the current thread is definitely the "game" thread. If in doubt returns false.
+ */
+bool IsGameThread();
+
 
 /**
  * Start a new thread.

src/video/video_driver.cpp

@@ -31,7 +31,7 @@ void VideoDriver::GameLoop()
 	if (this->next_game_tick < now - ALLOWED_DRIFT * this->GetGameInterval()) this->next_game_tick = now;
 
 	{
-		std::lock_guard<std::mutex> lock(this->game_state_mutex);
+		std::lock_guard<std::recursive_mutex> lock(this->game_state_mutex);
 
 		::GameLoop();
 	}
@@ -75,8 +75,23 @@ void VideoDriver::GameLoopPause()
 	this->game_state_mutex.lock();
 }
 
+/* static */ bool VideoDriver::EmergencyAcquireGameLock(uint tries, uint delay_ms)
+{
+	VideoDriver *drv = VideoDriver::GetInstance();
+	if (drv == nullptr) return true;
+
+
+	for (uint i = 0; i < tries; i++) {
+		if (drv->game_state_mutex.try_lock()) return true;
+		CSleep(delay_ms);
+	}
+
+	return false;
+}
+
 /* static */ void VideoDriver::GameThreadThunk(VideoDriver *drv)
 {
+	SetSelfAsGameThread();
 	drv->GameThread();
 }
 
@@ -135,7 +150,7 @@ void VideoDriver::Tick()
 		{
 			/* Tell the game-thread to stop so we can have a go. */
 			std::lock_guard<std::mutex> lock_wait(this->game_thread_wait_mutex);
-			std::lock_guard<std::mutex> lock_state(this->game_state_mutex);
+			std::lock_guard<std::recursive_mutex> lock_state(this->game_state_mutex);
 
 			this->next_draw_tick += this->GetDrawInterval();
 			/* Avoid next_draw_tick getting behind more and more if it cannot keep up. */

src/video/video_driver.hpp

@@ -212,6 +212,8 @@ public:
 		bool unlock; ///< Stores if the lock did anything that has to be undone.
 	};
 
+	static bool EmergencyAcquireGameLock(uint tries, uint delay_ms);
+
 protected:
 	const uint ALLOWED_DRIFT = 5; ///< How many times videodriver can miss deadlines without it being overly compensated.
 
@@ -322,7 +324,7 @@ protected:
 
 	bool is_game_threaded;
 	std::thread game_thread;
-	std::mutex game_state_mutex;
+	std::recursive_mutex game_state_mutex;
 	std::mutex game_thread_wait_mutex;
 
 	static void GameThreadThunk(VideoDriver *drv);