Skip to content

Commit 5806e3c

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

File tree

2 files changed

+20
-4
lines changed

2 files changed

+20
-4
lines changed

core/io/resource_loader.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,14 @@ 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 the fact that any resource loaded when an outer stack frame is
671+
// loading another one is considered a dependency of it, for progress
672+
// tracking purposes, a cycle can happen if even if the original resource
673+
// graphs involved have none. For instance, preload() can cause this.
674+
return load_task.max_reported_progress;
675+
}
676+
load_task.in_progress_check = true;
669677
float current_progress = 0.0;
670678
int dep_count = load_task.sub_tasks.size();
671679
if (dep_count > 0) {
@@ -679,6 +687,7 @@ float ResourceLoader::_dependency_get_progress(const String &p_path) {
679687
current_progress = load_task.progress;
680688
}
681689
load_task.max_reported_progress = MAX(load_task.max_reported_progress, current_progress);
690+
load_task.in_progress_check = false;
682691
return load_task.max_reported_progress;
683692
} else {
684693
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)