From f3e96a85485f68ee8c3e669e4bb6c910762a9130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Thu, 9 Nov 2023 12:29:47 +0100 Subject: [PATCH] Make languages bookkeeping thread-safe --- core/object/script_language.cpp | 34 ++++++++++++++++++++++++------ core/object/script_language.h | 7 +++--- core/object/worker_thread_pool.cpp | 9 ++++++++ core/object/worker_thread_pool.h | 1 + 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/core/object/script_language.cpp b/core/object/script_language.cpp index 011f4203ea7..2bdbfb5ad17 100644 --- a/core/object/script_language.cpp +++ b/core/object/script_language.cpp @@ -39,10 +39,11 @@ ScriptLanguage *ScriptServer::_languages[MAX_LANGUAGES]; int ScriptServer::_language_count = 0; +bool ScriptServer::languages_ready = false; +Mutex ScriptServer::languages_mutex; bool ScriptServer::scripting_enabled = true; bool ScriptServer::reload_scripts_on_save = false; -SafeFlag ScriptServer::languages_finished; // Used until GH-76581 is fixed properly. ScriptEditRequestFunction ScriptServer::edit_request_func = nullptr; void Script::_notification(int p_what) { @@ -160,12 +161,13 @@ bool ScriptServer::is_scripting_enabled() { } ScriptLanguage *ScriptServer::get_language(int p_idx) { + MutexLock lock(languages_mutex); ERR_FAIL_INDEX_V(p_idx, _language_count, nullptr); - return _languages[p_idx]; } Error ScriptServer::register_language(ScriptLanguage *p_language) { + MutexLock lock(languages_mutex); ERR_FAIL_NULL_V(p_language, ERR_INVALID_PARAMETER); ERR_FAIL_COND_V_MSG(_language_count >= MAX_LANGUAGES, ERR_UNAVAILABLE, "Script languages limit has been reach, cannot register more."); for (int i = 0; i < _language_count; i++) { @@ -179,6 +181,8 @@ Error ScriptServer::register_language(ScriptLanguage *p_language) { } Error ScriptServer::unregister_language(const ScriptLanguage *p_language) { + MutexLock lock(languages_mutex); + for (int i = 0; i < _language_count; i++) { if (_languages[i] == p_language) { _language_count--; @@ -219,17 +223,31 @@ void ScriptServer::init_languages() { } } - for (int i = 0; i < _language_count; i++) { - _languages[i]->init(); + { + MutexLock lock(languages_mutex); + + for (int i = 0; i < _language_count; i++) { + _languages[i]->init(); + } + + languages_ready = true; } } void ScriptServer::finish_languages() { + MutexLock lock(languages_mutex); + for (int i = 0; i < _language_count; i++) { _languages[i]->finish(); } global_classes_clear(); - languages_finished.set(); + + languages_ready = false; +} + +bool ScriptServer::are_languages_initialized() { + MutexLock lock(languages_mutex); + return languages_ready; } void ScriptServer::set_reload_scripts_on_save(bool p_enable) { @@ -241,7 +259,8 @@ bool ScriptServer::is_reload_scripts_on_save_enabled() { } void ScriptServer::thread_enter() { - if (!languages_finished.is_set()) { + MutexLock lock(languages_mutex); + if (!languages_ready) { return; } for (int i = 0; i < _language_count; i++) { @@ -250,7 +269,8 @@ void ScriptServer::thread_enter() { } void ScriptServer::thread_exit() { - if (!languages_finished.is_set()) { + MutexLock lock(languages_mutex); + if (!languages_ready) { return; } for (int i = 0; i < _language_count; i++) { diff --git a/core/object/script_language.h b/core/object/script_language.h index 3e4041d1733..85e64c8d62b 100644 --- a/core/object/script_language.h +++ b/core/object/script_language.h @@ -52,9 +52,11 @@ class ScriptServer { static ScriptLanguage *_languages[MAX_LANGUAGES]; static int _language_count; + static bool languages_ready; + static Mutex languages_mutex; + static bool scripting_enabled; static bool reload_scripts_on_save; - static SafeFlag languages_finished; // Used until GH-76581 is fixed properly. struct GlobalScriptClass { StringName language; @@ -98,8 +100,7 @@ public: static void init_languages(); static void finish_languages(); - - static bool are_languages_finished() { return languages_finished.is_set(); } + static bool are_languages_initialized(); }; class PlaceHolderScriptInstance; diff --git a/core/object/worker_thread_pool.cpp b/core/object/worker_thread_pool.cpp index 2fcd0867e6e..784acadab45 100644 --- a/core/object/worker_thread_pool.cpp +++ b/core/object/worker_thread_pool.cpp @@ -30,6 +30,7 @@ #include "worker_thread_pool.h" +#include "core/object/script_language.h" #include "core/os/os.h" #include "core/os/thread_safe.h" @@ -60,6 +61,14 @@ void WorkerThreadPool::_process_task(Task *p_task) { set_current_thread_safe_for_nodes(false); pool_thread_index = thread_ids[Thread::get_caller_id()]; ThreadData &curr_thread = threads[pool_thread_index]; + // Since the WorkerThreadPool is started before the script server, + // its pre-created threads can't have ScriptServer::thread_enter() called on them early. + // Therefore, we do it late at the first opportunity, so in case the task + // about to be run uses scripting, guarantees are held. + if (!curr_thread.ready_for_scripting && ScriptServer::are_languages_initialized()) { + ScriptServer::thread_enter(); + curr_thread.ready_for_scripting = true; + } task_mutex.lock(); p_task->pool_thread_index = pool_thread_index; if (low_priority) { diff --git a/core/object/worker_thread_pool.h b/core/object/worker_thread_pool.h index d4d93877658..f323a979f7f 100644 --- a/core/object/worker_thread_pool.h +++ b/core/object/worker_thread_pool.h @@ -106,6 +106,7 @@ private: uint32_t index; Thread thread; Task *current_low_prio_task = nullptr; + bool ready_for_scripting = false; }; TightLocalVector threads;