[Regression] LoadThreadedGetStatus stuck in ThreadLoadStatus.InProgress forever

[conde2]

Tested versions

Bug found in: v4.3.beta1.mono.official [a4f2ea9]

Previous working in: Godot v4.3.dev6.mono

System information

Godot v4.3.beta1.mono - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 Ti (NVIDIA; 31.0.15.3623) - AMD Ryzen 7 3700X 8-Core Processor (16 Threads)

Issue description

Trying to use ResourceLoader.LoadThreadedGetStatus always return ThreadLoadStatus.InProgress
My debug points that even when progress is 1 the status is ThreadLoadStatus.InProgress

Steps to reproduce

You need to have a TileMapLayer node with a TileSet and the TileSet must have an image within it.

Use the minimal reproduction project below to test it, see that it never leaves the while.

Minimal reproduction project (MRP)

bug-thread.zip

[raulsntos]

I can reproduce in v4.3.beta1.official [a4f2ea9] with the MRP ported to GDScript:

bug-thread.gdscript.zip

[matheusmdx]

Bisecting points to #91630 as the culprit:

[HKunogi]

Just to add to this, i dont know if its an actual design choice or a bug, but, lets say you call LoadThreadedRequest on asset X from code A, then on somewhere else, at code B, you also call LoadThreadedRequest on same asset X, after that, whoever of the code A or B, after the loading is complete, calls LoadThreadedGet on asset X first, will get the asset, but the second one (and all subsequents), will get an error stating its an invalid asset, requiring it to call LoadThreadedRequest on asset X yet again. Basically a LoadThreadedRequest on given asset can only be LoadThreadedGet once, so if several places calls the same asset to be loaded, only the first to call the get after its loaded will get it, and all others will need to request yet again.

[RandomShaper]

Just to add to this, i dont know if its an actual design choice or a bug, but, lets say you call LoadThreadedRequest on asset X from code A, then on somewhere else, at code B, you also call LoadThreadedRequest on same asset X, after that, whoever of the code A or B, after the loading is complete, calls LoadThreadedGet on asset X first, will get the asset, but the second one (and all subsequents), will get an error stating its an invalid asset, requiring it to call LoadThreadedRequest on asset X yet again. Basically a LoadThreadedRequest on given asset can only be LoadThreadedGet once, so if several places calls the same asset to be loaded, only the first to call the get after its loaded will get it, and all others will need to request yet again.

That would be a bug. Threaded load requesters don't need to be aware of others. One request and it can get, independently from others doing the same in parallel or not. When a threaded load is requested, a load token is created internally with a reference count that keeps it alive until all the load results have been collected.

[RandomShaper]

The problem here is that the loader thread needs to query the rendering server for some operation performed by the TileSet. For that to work, one of the following must be met:

• The rendering threading model is the separate thread one (so there's a thread processing requests in parallel).
• In single-safe thread mode (the default and only stable for now), engine iterations are allowed between the threaded load API calls, so that RenderingServer.sync() is called, to flush the command queue, letting the loader thread continue. For instance, doing the progress checks in _process().
• Keeping everything in _ready(), but performing the aforementioned sync explicitly; i.e., calling RenderingServer.force_sync() in the progress check loop.

That #91630 triggers a regression with the usage of the API in the MRP is just a symptom. Even if it was reverted, resources could still query the rendering server as part of they being loaded, during the first phase, before the command queue is involved. The issue would still be there, despite being harder to hit. It's a matter of how the engine is designed.

In short, my take is this is a documentation issue. UPDATE: I just learned the right usage is already documented.

@raulsntos, thanks for the GDScript MRP.

[RandomShaper]

Tested it indeed works in dev6, because #91630 wasn't still there, but, as explained above, it's more a lucky strike than a guaranteed behavior.

[conde2]

I can confirm that adding RenderingServer.ForceSync(); fix the stuck problem, but I belive it should be more well documented in the ResourceLoader.LoadThreadedGetStatus method description.

Chcecking the docs I couldn't find force_sync on the ResourceLoader page.

https://docs.godotengine.org/en/stable/classes/class_resourceloader.html#class-resourceloader-method-load-threaded-get-status

Its also not clear to me in which ocasions I should use it.