瀏覽代碼

Fix various race conditions with capturing of script backtraces

Mikael Hermansson 3 月之前
父節點
當前提交
6929823838

+ 1 - 9
core/error/error_macros.cpp

@@ -92,15 +92,7 @@ void _err_print_error(const char *p_function, const char *p_file, int p_line, co
 // Main error printing function.
 void _err_print_error(const char *p_function, const char *p_file, int p_line, const char *p_error, const char *p_message, bool p_editor_notify, ErrorHandlerType p_type) {
 	if (OS::get_singleton()) {
-		Vector<Ref<ScriptBacktrace>> script_backtraces;
-
-		// If script languages aren't initialized, we could be in the process of shutting down, in which case we don't want to allocate any objects, as we could be
-		// logging ObjectDB leaks, where ObjectDB would be locked, thus causing a deadlock.
-		if (ScriptServer::are_languages_initialized()) {
-			script_backtraces = ScriptServer::capture_script_backtraces(false);
-		}
-
-		OS::get_singleton()->print_error(p_function, p_file, p_line, p_error, p_message, p_editor_notify, (Logger::ErrorType)p_type, script_backtraces);
+		OS::get_singleton()->print_error(p_function, p_file, p_line, p_error, p_message, p_editor_notify, (Logger::ErrorType)p_type, ScriptServer::capture_script_backtraces(false));
 	} else {
 		// Fallback if errors happen before OS init or after it's destroyed.
 		const char *err_details = (p_message && *p_message) ? p_message : p_error;

+ 22 - 5
core/object/script_language.cpp

@@ -45,6 +45,15 @@ bool ScriptServer::scripting_enabled = true;
 bool ScriptServer::reload_scripts_on_save = false;
 ScriptEditRequestFunction ScriptServer::edit_request_func = nullptr;
 
+// These need to be the last static variables in this file, since we're exploiting the reverse-order destruction of static variables.
+static bool is_program_exiting = false;
+struct ProgramExitGuard {
+	~ProgramExitGuard() {
+		is_program_exiting = true;
+	}
+};
+static ProgramExitGuard program_exit_guard;
+
 void Script::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_POSTINITIALIZE: {
@@ -536,13 +545,21 @@ void ScriptServer::save_global_classes() {
 }
 
 Vector<Ref<ScriptBacktrace>> ScriptServer::capture_script_backtraces(bool p_include_variables) {
-	int language_count = ScriptServer::get_language_count();
+	if (is_program_exiting) {
+		return Vector<Ref<ScriptBacktrace>>();
+	}
+
+	MutexLock lock(languages_mutex);
+	if (!languages_ready) {
+		return Vector<Ref<ScriptBacktrace>>();
+	}
+
 	Vector<Ref<ScriptBacktrace>> result;
-	result.resize(language_count);
-	for (int i = 0; i < language_count; i++) {
-		ScriptLanguage *language = ScriptServer::get_language(i);
-		result.write[i].instantiate(language, p_include_variables);
+	result.resize(_language_count);
+	for (int i = 0; i < _language_count; i++) {
+		result.write[i].instantiate(_languages[i], p_include_variables);
 	}
+
 	return result;
 }
 

+ 5 - 8
platform/linuxbsd/crash_handler_linuxbsd.cpp

@@ -146,14 +146,11 @@ static void handle_crash(int sig) {
 	print_error("-- END OF C++ BACKTRACE --");
 	print_error("================================================================");
 
-	if (ScriptServer::are_languages_initialized()) {
-		Vector<Ref<ScriptBacktrace>> script_backtraces = ScriptServer::capture_script_backtraces(false);
-		for (const Ref<ScriptBacktrace> &backtrace : script_backtraces) {
-			if (!backtrace->is_empty()) {
-				print_error(backtrace->format());
-				print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
-				print_error("================================================================");
-			}
+	for (const Ref<ScriptBacktrace> &backtrace : ScriptServer::capture_script_backtraces(false)) {
+		if (!backtrace->is_empty()) {
+			print_error(backtrace->format());
+			print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
+			print_error("================================================================");
 		}
 	}
 

+ 5 - 8
platform/macos/crash_handler_macos.mm

@@ -176,14 +176,11 @@ static void handle_crash(int sig) {
 	print_error("-- END OF C++ BACKTRACE --");
 	print_error("================================================================");
 
-	if (ScriptServer::are_languages_initialized()) {
-		Vector<Ref<ScriptBacktrace>> script_backtraces = ScriptServer::capture_script_backtraces(false);
-		for (const Ref<ScriptBacktrace> &backtrace : script_backtraces) {
-			if (!backtrace->is_empty()) {
-				print_error(backtrace->format());
-				print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
-				print_error("================================================================");
-			}
+	for (const Ref<ScriptBacktrace> &backtrace : ScriptServer::capture_script_backtraces(false)) {
+		if (!backtrace->is_empty()) {
+			print_error(backtrace->format());
+			print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
+			print_error("================================================================");
 		}
 	}
 

+ 5 - 8
platform/windows/crash_handler_windows_seh.cpp

@@ -230,14 +230,11 @@ DWORD CrashHandlerException(EXCEPTION_POINTERS *ep) {
 
 	SymCleanup(process);
 
-	if (ScriptServer::are_languages_initialized()) {
-		Vector<Ref<ScriptBacktrace>> script_backtraces = ScriptServer::capture_script_backtraces(false);
-		for (const Ref<ScriptBacktrace> &backtrace : script_backtraces) {
-			if (!backtrace->is_empty()) {
-				print_error(backtrace->format());
-				print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
-				print_error("================================================================");
-			}
+	for (const Ref<ScriptBacktrace> &backtrace : ScriptServer::capture_script_backtraces(false)) {
+		if (!backtrace->is_empty()) {
+			print_error(backtrace->format());
+			print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
+			print_error("================================================================");
 		}
 	}
 

+ 5 - 8
platform/windows/crash_handler_windows_signal.cpp

@@ -184,14 +184,11 @@ extern void CrashHandlerException(int signal) {
 	print_error("-- END OF C++ BACKTRACE --");
 	print_error("================================================================");
 
-	if (ScriptServer::are_languages_initialized()) {
-		Vector<Ref<ScriptBacktrace>> script_backtraces = ScriptServer::capture_script_backtraces(false);
-		for (const Ref<ScriptBacktrace> &backtrace : script_backtraces) {
-			if (!backtrace->is_empty()) {
-				print_error(backtrace->format());
-				print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
-				print_error("================================================================");
-			}
+	for (const Ref<ScriptBacktrace> &backtrace : ScriptServer::capture_script_backtraces(false)) {
+		if (!backtrace->is_empty()) {
+			print_error(backtrace->format());
+			print_error(vformat("-- END OF %s BACKTRACE --", backtrace->get_language_name().to_upper()));
+			print_error("================================================================");
 		}
 	}
 }