From 41180d1ee53a14a514ad601239c731a8c48cd912 Mon Sep 17 00:00:00 2001 From: Aleksey Vasenev Date: Mon, 29 Jul 2024 20:20:55 +0300 Subject: [PATCH] Fix use condition_variable after free --- core/io/resource_loader.cpp | 19 +++++++++++-------- core/io/resource_loader.h | 2 ++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp index d606db620c19..8f85770fd595 100644 --- a/core/io/resource_loader.cpp +++ b/core/io/resource_loader.cpp @@ -337,11 +337,10 @@ void ResourceLoader::_thread_load_function(void *p_userdata) { load_task.status = THREAD_LOAD_LOADED; } - if (load_task.cond_var) { + if (load_task.cond_var && load_task.need_wait) { load_task.cond_var->notify_all(); - memdelete(load_task.cond_var); - load_task.cond_var = nullptr; } + load_task.need_wait = false; bool ignoring = load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE || load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE_DEEP; bool replacing = load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE || load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE_DEEP; @@ -716,15 +715,20 @@ Ref ResourceLoader::_load_complete_inner(LoadToken &p_load_token, Erro DEV_ASSERT(wtp_task_err == OK); thread_load_mutex.lock(); } - } else { + } else if (load_task.need_wait) { // Loading thread is main or user thread. if (!load_task.cond_var) { load_task.cond_var = memnew(ConditionVariable); } + ++load_task.awaiters_count; do { load_task.cond_var->wait(p_thread_load_lock); DEV_ASSERT(thread_load_tasks.has(p_load_token.local_path) && p_load_token.get_reference_count()); - } while (load_task.cond_var); + } while (load_task.need_wait); + if (!--load_task.awaiters_count) { + memdelete(load_task.cond_var); + load_task.cond_var = nullptr; + } } } else { if (loader_is_wtp) { @@ -1152,11 +1156,10 @@ void ResourceLoader::clear_thread_load_tasks() { if (thread_load_tasks.size()) { for (KeyValue &E : thread_load_tasks) { if (E.value.status == THREAD_LOAD_IN_PROGRESS) { - if (E.value.cond_var) { + if (E.value.cond_var && E.value.need_wait) { E.value.cond_var->notify_all(); - memdelete(E.value.cond_var); - E.value.cond_var = nullptr; } + E.value.need_wait = false; none_running = false; } } diff --git a/core/io/resource_loader.h b/core/io/resource_loader.h index 5f1831f0d988..538c7246b2c3 100644 --- a/core/io/resource_loader.h +++ b/core/io/resource_loader.h @@ -167,6 +167,8 @@ class ResourceLoader { Thread::ID thread_id = 0; // Used if running on an user thread (e.g., simple non-threaded load). bool awaited = false; // If it's in the pool, this helps not awaiting from more than one dependent thread. ConditionVariable *cond_var = nullptr; // In not in the worker pool or already awaiting, this is used as a secondary awaiting mechanism. + uint32_t awaiters_count = 0; + bool need_wait = true; LoadToken *load_token = nullptr; String local_path; String remapped_path;