Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.3] Merge commit godotengine/godot@711c725 #863

Merged
merged 25 commits into from
Nov 15, 2024

Conversation

Spartan322
Copy link
Member

No description provided.

RandomShaper and others added 25 commits September 5, 2024 13:24
…gy TLS

This reverts commit 41c0785.

(cherry picked from commit e9407d4)
Benefits:
- Simpler code. The main load function is renamed so it's apparent that it's not just a thread entry point anymore.
- Cache and thread modes of the original task are honored. A beautiful consequence of this is that, unlike formerly, re-issued loads can use the resource cache, which makes this mechanism much more performant.
- The newly added getter for caller task id in WorkerThreadPool allows to remove the custom tracking of that in ResourceLoader.
- The check to replace a cached resource and the replacement itself happen atomically. That fixes deadlock prevention leading to multiple resource instances of the same one on disk. As a side effect, it also makes the regular check for replace load mode more robust.

(cherry picked from commit 28619e2)
1. Make handling of user tokens atomic:
   Loads started with the external-facing API used to perform a two-step setup of the user token. Between both, the mutex was unlocked without its reference count having been increased. A non-user-initiated load could therefore destroy the load task when it unreferenced the token.
   Those stages now happen atomically so in the one hand, the described race condition can't happen so the load task life insurance doesn't have a gap anymore and, on the other hand, the ugliness that the call to load could return `ERR_BUSY` if happening while other thread was between both steps is gone.
   The code has been refactored so the user token concerns are still outside the inner load start function, which is agnostic to that for a cleaner implementation.
2. Clear ambiguity between load operations running on `WorkerThreadPool`:
   The two cases are: single-loaded thread directly started by a user pool task and a load started by the system as part of a multi-threaded load.
   Since ensuring all the code dealing with this distinction would make it very complex, and error-prone, a different measure is applied instead: just take one of the cases out of the dicotomy. We now ensure every load happening on a pool thread has been initiated by the system.
   The way of achieving that is that a single-threaded user-started load initiated from a pool thread, is run as another task.

(cherry picked from commit df23858)
This fixes a rare but possible deadlock, maybe due to undefined behavior. The new implementation is safer, at the cost of some added boilerplate.

(cherry picked from commit f4d7685)
(cherry picked from commit 0f3ee922e07fd4d16d9ef6dac150beb9c84ac527)
This is a complement to: godotengine/godot#96593

(cherry picked from commit 97197ff)
Co-authored-by: Summersay415 <[email protected]>

(cherry picked from commit fe21913)
`ClassDB::can_instantiate()` and other reflection methods deadlock if the type is an script global class, when such script indirectly uses a not-yet-registered class. The reason is the `ClassDB` read lock is still held when invoking the `ResourceLoader` to load the class script, which may in turn need to lock for writing (for the class registration).

In particular, this happens with some types related to animation tree, that aren't registered at engine startup, but can happen with others, especially ones from the user. Registration statements are also added for the animation-related types that were lacking them.
fixes animation snapping rounding to weird values in seconds mode + fixes holding shift not reducing snap value
[4.3] Cherry-picks related to `ResourceLoader`
[4.3] Fix deadlocks related to ClassDB queries about global classes
…_fix

[4.3] Fix animation snapping in seconds mode
[4.3] Windows: Avoid child processes inheriting all file handles
@Spartan322 Spartan322 merged commit a0eabe2 into Redot-Engine:4.3 Nov 15, 2024
18 checks passed
@Spartan322 Spartan322 deleted the 4.3-merge/711c725 branch November 15, 2024 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants