Skip to content

Commit 3596d75

Browse files
committed
ResourceLoader: Fix potential infinite recursion in progress reporting
1 parent 235a32a commit 3596d75

File tree

2 files changed

+19
-4
lines changed

2 files changed

+19
-4
lines changed

core/io/resource_loader.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,13 @@ Ref<ResourceLoader::LoadToken> ResourceLoader::_load_start(const String &p_path,
666666
float ResourceLoader::_dependency_get_progress(const String &p_path) {
667667
if (thread_load_tasks.has(p_path)) {
668668
ThreadLoadTask &load_task = thread_load_tasks[p_path];
669+
if (load_task.in_progress_check) {
670+
// Given this algorithm has a global view of the resource loads in flight,
671+
// it may work across the boundaries of individual resource graphs. Because
672+
// of that, cycles can appear depite those graphs themselves not having any.
673+
return load_task.max_reported_progress;
674+
}
675+
load_task.in_progress_check = true;
669676
float current_progress = 0.0;
670677
int dep_count = load_task.sub_tasks.size();
671678
if (dep_count > 0) {
@@ -679,6 +686,7 @@ float ResourceLoader::_dependency_get_progress(const String &p_path) {
679686
current_progress = load_task.progress;
680687
}
681688
load_task.max_reported_progress = MAX(load_task.max_reported_progress, current_progress);
689+
load_task.in_progress_check = false;
682690
return load_task.max_reported_progress;
683691
} else {
684692
return 1.0; //assume finished loading it so it no longer exists

core/io/resource_loader.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,8 @@ class ResourceLoader {
176176
struct ThreadLoadTask {
177177
WorkerThreadPool::TaskID task_id = 0; // Used if run on a worker thread from the pool.
178178
Thread::ID thread_id = 0; // Used if running on an user thread (e.g., simple non-threaded load).
179-
bool awaited = false; // If it's in the pool, this helps not awaiting from more than one dependent thread.
180179
ConditionVariable *cond_var = nullptr; // In not in the worker pool or already awaiting, this is used as a secondary awaiting mechanism.
181180
uint32_t awaiters_count = 0;
182-
bool need_wait = true;
183181
LoadToken *load_token = nullptr;
184182
String local_path;
185183
String type_hint;
@@ -190,17 +188,26 @@ class ResourceLoader {
190188
ResourceFormatLoader::CacheMode cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE;
191189
Error error = OK;
192190
Ref<Resource> resource;
193-
bool use_sub_threads = false;
194191
HashSet<String> sub_tasks;
195192

193+
bool awaited : 1; // If it's in the pool, this helps not awaiting from more than one dependent thread.
194+
bool need_wait : 1;
195+
bool in_progress_check : 1; // Measure against recursion cycles in progress reporting. Cycles are not expected, but can happen due to how it's currently implemented.
196+
bool use_sub_threads : 1;
197+
196198
struct ResourceChangedConnection {
197199
Resource *source = nullptr;
198200
Callable callable;
199201
uint32_t flags = 0;
200202
};
201203
LocalVector<ResourceChangedConnection> resource_changed_connections;
202-
};
203204

205+
ThreadLoadTask() :
206+
awaited(false),
207+
need_wait(true),
208+
in_progress_check(false),
209+
use_sub_threads(false) {}
210+
};
204211
static void _run_load_task(void *p_userdata);
205212

206213
static thread_local bool import_thread;

0 commit comments

Comments
 (0)