Merge pull request #96760 from RandomShaper/wtp_langs_exit_thread

Make use of languages' thread enter/exit more correct

core/object/script_language.cpp

@@ -41,6 +41,7 @@ ScriptLanguage *ScriptServer::_languages[MAX_LANGUAGES];
 int ScriptServer::_language_count = 0;
 bool ScriptServer::languages_ready = false;
 Mutex ScriptServer::languages_mutex;
+thread_local bool ScriptServer::thread_entered = false;
 
 bool ScriptServer::scripting_enabled = true;
 bool ScriptServer::reload_scripts_on_save = false;
@@ -326,6 +327,10 @@ bool ScriptServer::are_languages_initialized() {
 	return languages_ready;
 }
 
+bool ScriptServer::thread_is_entered() {
+	return thread_entered;
+}
+
 void ScriptServer::set_reload_scripts_on_save(bool p_enable) {
 	reload_scripts_on_save = p_enable;
 }
@@ -335,6 +340,10 @@ bool ScriptServer::is_reload_scripts_on_save_enabled() {
 }
 
 void ScriptServer::thread_enter() {
+	if (thread_entered) {
+		return;
+	}
+
 	MutexLock lock(languages_mutex);
 	if (!languages_ready) {
 		return;
@@ -342,9 +351,15 @@ void ScriptServer::thread_enter() {
 	for (int i = 0; i < _language_count; i++) {
 		_languages[i]->thread_enter();
 	}
+
+	thread_entered = true;
 }
 
 void ScriptServer::thread_exit() {
+	if (!thread_entered) {
+		return;
+	}
+
 	MutexLock lock(languages_mutex);
 	if (!languages_ready) {
 		return;
@@ -352,6 +367,8 @@ void ScriptServer::thread_exit() {
 	for (int i = 0; i < _language_count; i++) {
 		_languages[i]->thread_exit();
 	}
+
+	thread_entered = false;
 }
 
 HashMap<StringName, ScriptServer::GlobalScriptClass> ScriptServer::global_classes;

core/object/script_language.h

@@ -54,6 +54,7 @@ class ScriptServer {
 	static int _language_count;
 	static bool languages_ready;
 	static Mutex languages_mutex;
+	static thread_local bool thread_entered;
 
 	static bool scripting_enabled;
 	static bool reload_scripts_on_save;
@@ -101,6 +102,7 @@ public:
 	static void init_languages();
 	static void finish_languages();
 	static bool are_languages_initialized();
+	static bool thread_is_entered();
 };
 
 class PlaceHolderScriptInstance;

core/object/worker_thread_pool.cpp

@@ -63,17 +63,14 @@ void WorkerThreadPool::_process_task(Task *p_task) {
 		// Tasks must start with these at default values. They are free to set-and-forget otherwise.
 		set_current_thread_safe_for_nodes(false);
 		MessageQueue::set_thread_singleton_override(nullptr);
+
 		// 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.
+		ScriptServer::thread_enter();
+
 		task_mutex.lock();
-		if (!curr_thread.ready_for_scripting && ScriptServer::are_languages_initialized()) {
-			task_mutex.unlock();
-			ScriptServer::thread_enter();
-			task_mutex.lock();
-			curr_thread.ready_for_scripting = true;
-		}
 		p_task->pool_thread_index = pool_thread_index;
 		prev_task = curr_thread.current_task;
 		curr_thread.current_task = p_task;
@@ -326,6 +323,8 @@ WorkerThreadPool::TaskID WorkerThreadPool::add_native_task(void (*p_func)(void *
 }
 
 WorkerThreadPool::TaskID WorkerThreadPool::_add_task(const Callable &p_callable, void (*p_func)(void *), void *p_userdata, BaseTemplateUserdata *p_template_userdata, bool p_high_priority, const String &p_description) {
+	ERR_FAIL_COND_V_MSG(threads.is_empty(), INVALID_TASK_ID, "Can't add a task because the WorkerThreadPool is either not initialized yet or already terminated.");
+
 	task_mutex.lock();
 	// Get a free task
 	Task *task = task_allocator.alloc();
@@ -514,6 +513,12 @@ void WorkerThreadPool::yield() {
 	int th_index = get_thread_index();
 	ERR_FAIL_COND_MSG(th_index == -1, "This function can only be called from a worker thread.");
 	_wait_collaboratively(&threads[th_index], ThreadData::YIELDING);
+
+	// If this long-lived task started before the scripting server was initialized,
+	// now is a good time to have scripting languages ready for the current thread.
+	// Otherwise, such a piece of setup won't happen unless another task has been
+	// run during the collaborative wait.
+	ScriptServer::thread_enter();
 }
 
 void WorkerThreadPool::notify_yield_over(TaskID p_task_id) {
@@ -538,6 +543,7 @@ void WorkerThreadPool::notify_yield_over(TaskID p_task_id) {
 }
 
 WorkerThreadPool::GroupID WorkerThreadPool::_add_group_task(const Callable &p_callable, void (*p_func)(void *, uint32_t), void *p_userdata, BaseTemplateUserdata *p_template_userdata, int p_elements, int p_tasks, bool p_high_priority, const String &p_description) {
+	ERR_FAIL_COND_V_MSG(threads.is_empty(), INVALID_TASK_ID, "Can't add a group task because the WorkerThreadPool is either not initialized yet or already terminated.");
 	ERR_FAIL_COND_V(p_elements < 0, INVALID_TASK_ID);
 	if (p_tasks < 0) {
 		p_tasks = MAX(1u, threads.size());
@@ -749,5 +755,5 @@ WorkerThreadPool::WorkerThreadPool() {
 }
 
 WorkerThreadPool::~WorkerThreadPool() {
-	finish();
+	DEV_ASSERT(threads.size() == 0 && "finish() hasn't been called!");
 }

core/object/worker_thread_pool.h

@@ -112,7 +112,6 @@ private:
 
 		uint32_t index = 0;
 		Thread thread;
-		bool ready_for_scripting : 1;
 		bool signaled : 1;
 		bool yield_is_over : 1;
 		Task *current_task = nullptr;
@@ -120,7 +119,6 @@ private:
 		ConditionVariable cond_var;
 
 		ThreadData() :
-				ready_for_scripting(false),
 				signaled(false),
 				yield_is_over(false) {}
 	};

core/register_core_types.cpp

@@ -107,8 +107,6 @@ static Time *_time = nullptr;
 static core_bind::Geometry2D *_geometry_2d = nullptr;
 static core_bind::Geometry3D *_geometry_3d = nullptr;
 
-static WorkerThreadPool *worker_thread_pool = nullptr;
-
 extern Mutex _global_mutex;
 
 static GDExtensionManager *gdextension_manager = nullptr;
@@ -297,8 +295,6 @@ void register_core_types() {
 	GDREGISTER_NATIVE_STRUCT(AudioFrame, "float left;float right");
 	GDREGISTER_NATIVE_STRUCT(ScriptLanguageExtensionProfilingInfo, "StringName signature;uint64_t call_count;uint64_t total_time;uint64_t self_time");
 
-	worker_thread_pool = memnew(WorkerThreadPool);
-
 	OS::get_singleton()->benchmark_end_measure("Core", "Register Types");
 }
 
@@ -349,7 +345,7 @@ void register_core_singletons() {
 	Engine::get_singleton()->add_singleton(Engine::Singleton("Time", Time::get_singleton()));
 	Engine::get_singleton()->add_singleton(Engine::Singleton("GDExtensionManager", GDExtensionManager::get_singleton()));
 	Engine::get_singleton()->add_singleton(Engine::Singleton("ResourceUID", ResourceUID::get_singleton()));
-	Engine::get_singleton()->add_singleton(Engine::Singleton("WorkerThreadPool", worker_thread_pool));
+	Engine::get_singleton()->add_singleton(Engine::Singleton("WorkerThreadPool", WorkerThreadPool::get_singleton()));
 
 	OS::get_singleton()->benchmark_end_measure("Core", "Register Singletons");
 }
@@ -382,8 +378,6 @@ void unregister_core_types() {
 
 	// Destroy singletons in reverse order to ensure dependencies are not broken.
 
-	memdelete(worker_thread_pool);
-
 	memdelete(_engine_debugger);
 	memdelete(_marshalls);
 	memdelete(_classdb);

main/main.cpp

@@ -140,6 +140,7 @@ static Engine *engine = nullptr;
 static ProjectSettings *globals = nullptr;
 static Input *input = nullptr;
 static InputMap *input_map = nullptr;
+static WorkerThreadPool *worker_thread_pool = nullptr;
 static TranslationServer *translation_server = nullptr;
 static Performance *performance = nullptr;
 static PackedData *packed_data = nullptr;
@@ -690,6 +691,8 @@ Error Main::test_setup() {
 
 	register_core_settings(); // Here globals are present.
 
+	worker_thread_pool = memnew(WorkerThreadPool);
+
 	translation_server = memnew(TranslationServer);
 	tsman = memnew(TextServerManager);
 
@@ -800,6 +803,8 @@ void Main::test_cleanup() {
 	ResourceSaver::remove_custom_savers();
 	PropertyListHelper::clear_base_helpers();
 
+	WorkerThreadPool::get_singleton()->finish();
+
 #ifdef TOOLS_ENABLED
 	GDExtensionManager::get_singleton()->deinitialize_extensions(GDExtension::INITIALIZATION_LEVEL_EDITOR);
 	uninitialize_modules(MODULE_INITIALIZATION_LEVEL_EDITOR);
@@ -841,6 +846,9 @@ void Main::test_cleanup() {
 	if (physics_server_2d_manager) {
 		memdelete(physics_server_2d_manager);
 	}
+	if (worker_thread_pool) {
+		memdelete(worker_thread_pool);
+	}
 	if (globals) {
 		memdelete(globals);
 	}
@@ -931,6 +939,7 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
 
 	register_core_settings(); //here globals are present
 
+	worker_thread_pool = memnew(WorkerThreadPool);
 	translation_server = memnew(TranslationServer);
 	performance = memnew(Performance);
 	GDREGISTER_CLASS(Performance);
@@ -2621,6 +2630,10 @@ error:
 	if (translation_server) {
 		memdelete(translation_server);
 	}
+	if (worker_thread_pool) {
+		worker_thread_pool->finish();
+		memdelete(worker_thread_pool);
+	}
 	if (globals) {
 		memdelete(globals);
 	}
@@ -4502,6 +4515,8 @@ void Main::cleanup(bool p_force) {
 	ResourceLoader::clear_translation_remaps();
 	ResourceLoader::clear_path_remaps();
 
+	WorkerThreadPool::get_singleton()->finish();
+
 	ScriptServer::finish_languages();
 
 	// Sync pending commands that may have been queued from a different thread during ScriptServer finalization
@@ -4592,6 +4607,9 @@ void Main::cleanup(bool p_force) {
 	if (physics_server_2d_manager) {
 		memdelete(physics_server_2d_manager);
 	}
+	if (worker_thread_pool) {
+		memdelete(worker_thread_pool);
+	}
 	if (globals) {
 		memdelete(globals);
 	}