Skip to content

Commit 8480dc2

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

File tree

2 files changed

+35
-10
lines changed

2 files changed

+35
-10
lines changed

core/io/resource_loader.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,11 @@ 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+
// Avoid cycles.
671+
return load_task.max_reported_progress;
672+
}
673+
load_task.in_progress_check = true;
669674
float current_progress = 0.0;
670675
int dep_count = load_task.sub_tasks.size();
671676
if (dep_count > 0) {
@@ -679,6 +684,7 @@ float ResourceLoader::_dependency_get_progress(const String &p_path) {
679684
current_progress = load_task.progress;
680685
}
681686
load_task.max_reported_progress = MAX(load_task.max_reported_progress, current_progress);
687+
load_task.in_progress_check = false;
682688
return load_task.max_reported_progress;
683689
} else {
684690
return 1.0; //assume finished loading it so it no longer exists

core/io/resource_loader.h

Lines changed: 29 additions & 10 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;
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;
@@ -228,7 +235,9 @@ class ResourceLoader {
228235
static ThreadLoadStatus load_threaded_get_status(const String &p_path, float *r_progress = nullptr);
229236
static Ref<Resource> load_threaded_get(const String &p_path, Error *r_error = nullptr);
230237

231-
static bool is_within_load() { return load_nesting > 0; }
238+
static bool is_within_load() {
239+
return load_nesting > 0;
240+
}
232241

233242
static void resource_changed_connect(Resource *p_source, const Callable &p_callable, uint32_t p_flags);
234243
static void resource_changed_disconnect(Resource *p_source, const Callable &p_callable);
@@ -253,8 +262,12 @@ class ResourceLoader {
253262

254263
static void set_is_import_thread(bool p_import_thread);
255264

256-
static void set_timestamp_on_load(bool p_timestamp) { timestamp_on_load = p_timestamp; }
257-
static bool get_timestamp_on_load() { return timestamp_on_load; }
265+
static void set_timestamp_on_load(bool p_timestamp) {
266+
timestamp_on_load = p_timestamp;
267+
}
268+
static bool get_timestamp_on_load() {
269+
return timestamp_on_load;
270+
}
258271

259272
// Loaders can safely use this regardless which thread they are running on.
260273
static void notify_load_error(const String &p_err) {
@@ -280,8 +293,12 @@ class ResourceLoader {
280293
dep_err_notify = p_err_notify;
281294
}
282295

283-
static void set_abort_on_missing_resources(bool p_abort) { abort_on_missing_resource = p_abort; }
284-
static bool get_abort_on_missing_resources() { return abort_on_missing_resource; }
296+
static void set_abort_on_missing_resources(bool p_abort) {
297+
abort_on_missing_resource = p_abort;
298+
}
299+
static bool get_abort_on_missing_resources() {
300+
return abort_on_missing_resource;
301+
}
285302

286303
static String path_remap(const String &p_path);
287304
static String import_remap(const String &p_path);
@@ -300,7 +317,9 @@ class ResourceLoader {
300317
static void remove_custom_loaders();
301318

302319
static void set_create_missing_resources_if_class_unavailable(bool p_enable);
303-
_FORCE_INLINE_ static bool is_creating_missing_resources_if_class_unavailable_enabled() { return create_missing_resources_if_class_unavailable; }
320+
_FORCE_INLINE_ static bool is_creating_missing_resources_if_class_unavailable_enabled() {
321+
return create_missing_resources_if_class_unavailable;
322+
}
304323

305324
static Ref<Resource> ensure_resource_ref_override_for_outer_load(const String &p_path, const String &p_res_type);
306325
static Ref<Resource> get_resource_ref_override(const String &p_path);

0 commit comments

Comments
 (0)