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.
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.
This switches to 64-bit integers in select locations of the Image
class, so that image resolutions of 16384×16384 (used by
lightmap texture arrays) can be used properly. Values that are larger
should also work.
VRAM compression is also supported, although most VRAM-compressed
formats are limited to individual slices of 16384×16384. WebP
is limited to 16383×16383 due to format limitations.
This is about not letting the resource format loader set the error code directly on the task anymore. Instead, it's stored locally and assigned only when it is right to do so.
Otherwise, other tasks may see an error code in the current one before it's state having transitioned to errored. While this, besides the technically true data race, may not be a problem in practice, it causes surprising situations during debugging as it breaks assumptions.
ResourceLoader:
- Fix invalid tokens being returned.
- Remove no longer written `ThreadLoadTask::dependent_path` and the code reading from it.
- Clear deadlock hazard by keeping the mutex unlocked during userland polling.
WorkerThreadPool:
- Include thread call queue override in the thread state reset set, which allows to simplify the code that handled that (imperfectly) in the ResourceLoader.
- Handle the mutex type correctly on entering an allowance zone.
CommandQueueMT:
- Handle the additional possibility of command buffer reallocation that mutex unlock allowance introduces.
These deferring measures were added to aid threaded resource loading in being safe.
They were removed as seemingly unneeded, but it seems they are needed so resources involved in threaded loading interact with others only after "sync points".
- Allows the message queue override to flush after loading each resource, which was the original intent.
- Removes a redundant call to mark the thread as safe-for-nodes.
- `CACHE_MODE_IGNORE_DEEP` is checked in addition to `CACHE_MODE_IGNORE` to determine if a load is uncached. This avoids crashes in uncached loads due to prematurely freed load tasks.
- Cached load tasks are isolated (not registered in the task map ever). This avoids regular loads from reusing in-flight cached loads, which is not correct.
* Replaces `find(...) != -1` with `contains` for `String`
* Replaces `find(...) == -1` with `!contains` for `String`
* Replaces `find(...) != -1` with `has` for containers
* Replaces `find(...) == -1` with `!has` for containers
Random-access access to `List` when iterating is `O(n^2)` (`O(n)` when
accessing a single element)
* Removed subscript operator, in favor of a more explicit `get`
* Added conversion from `Iterator` to `ConstIterator`
* Remade existing operations into other solutions when applicable
Text editors cannot display ANSI escape codes, so these should be
stripped from log files to ensure readability.
Since this uses a regex, this works both for `print_rich()`
and manually inserted ANSI escape codes.
Co-authored-by: bruvzg <7645683+bruvzg@users.noreply.github.com>
* Allow typed arrays to be encoded even if `p_full_objects` is `false`.
* Use `Array[EncodedObjectAsID]` instead of the encoded element type
when decoding an array if `p_allow_objects` is `false`.
If the `StreamPeerTCP` is polled and the TCP connection is `STATUS_CONNECTED` it should return after polling netsocket. Without `return` poll keeps calling `_sock->connect_to_host` and `connect()`.
We changed how scripts are binary serialized, and added a check to
ensure the new format is enforced, but there is still a case where the
old format (plain "prop"-"value" combo) is used, and that is when the
script is NIL.
fixes godotengine#82061
fixes godotengine#61556
Also, distinguish between main pack and DLC packs.
It's desirable to downloaded content to be as small as possible. This change avoids bloating non-main pack files with new versions of resources that are all read on startup and never used again. They have no effect if loaded after startup.
- project.godot/project.binary file
- extension_list.cfg
- app icon and boot_splash
- .ico and .icns files (these can still be opted in for DLC by listing them explicitly in the include filter)
- Unify documentation, hoping to clear misconcepctions about about propagation of the cache mode across dependant loads.
- Clarify in docs that `CACHE_MODE_REPLACE` now also works on the main resource (from #87008).
- Add two recursive modes, counterparts of `CACHE_MODE_REPLACE` and `CACHE_MODE_IGNORE`, since it seems some need them (see #59669, #82830).
- Let resources, even loaded with one of the ignore-cache modes, get a path, which is useful for tools.
This change simply extracts 'SafeBinaryMutex' from 'mutex.h' to
'safe_binary_mutex.h', in an effort to reduce the compilation
speed impact of including `mutex.h`.
This also makes errors related to asset image loading
verbose-only, because, frankly, users can't do much about
those errors. Spamming them with error messages
about some assets on the frontend being broken
is pointless.
Any resource that contains other local to scene resources inside of
arrays or dictionaries will now be duplicated and configured.
The case where a scene's node has an exported array/dictionary
property containing local to scene resources is NOT handled here.
This commits rewrites the sync logic in a way that the
`use_system_threads_for_low_priority_tasks` setting, which was added due to
the lack of a cross-platform wait-for-multiple-objects functionality, can be
removed (it's as if it was effectively hardcoded to `false`).
With the new implementation, we have the best of both worlds: threads don't
have to poll, plus no bespoke threads are used.
In addition, regarding deadlock prevention, since not every possible case of
wait-deadlock could be avoided, this commits removes the current best-effort
avoidance mechanisms and keeps only a simple, pessimistic way of detection.
It turns out that the only current user of deadlock prevention, ResourceLoader,
works fine with it and so every possible situation in resource loading is now
properly handled, with no possibilities of deadlocking. There's a comment in
the code with further details.
Lastly, a potential for load tasks never being awaited/disposed is cleared.
I was trying to import a solid colored metal/roughness texture that
worked in Godot 4.1.1 but failed to load on master. I eventually tracked
it down to this: to_x and to_y can be 0, which then leads them to be -1
later, which then causes ofs to overflow.
Some of these cases were hidden because of the use of Vector so we were
just happily scribbling in ram elsewhere.
Switched to LocalVector as it made debugging a bit easier.
This allows to include script_instance.h directly in the
generated gdvirtual.gen.inc, and remove excessive includes
from the codebase.
This should also allow Resource to use GDVIRTUAL macros,
which wasn't possible previously due to a circular dependency.
PCK files (like other build products) should be deterministic based on their inputs. Removed calls to Math::rand() that are being used to generate padding.
Looks like these were introduced as part of adding encryption support, but the padding being random does not have any cryptographic significance. This can be trivially inferred since file blocks that happen to be aligned don't get padding anyway.
If there's a desire to indroduce something that functions as a nonce it should probably be added explicitly and only if encryption is enabled.
remove Math::rand() calls in editor_export_platform.cpp
follow up to make consistent with pck_packer
Saving a subscene causes the main scene to be re-instantiated. And the resource
instance in the main scene will be reused when the main scene is re-instantiated.
So for resources with `resource_local_to_scene` enabled, resetting state may be
necessary (at least for `ViewportTexture`).
When saving resources, marking of already seen resources was
done too late, causing infinite loop traversing referenced resources
and eventual stack overflow. The change marks traversed resource
before descending to it's children, thus when this resource is
encountered again, it is already marked as seen and traversal stops.
Rémi Verschelde
Pedro J. Estébanez
Rémi Verschelde
Hilderin
Daylily-Zeleen
BlueCube3310
Haoyu Qiu
Summersay415
Mikael Hermansson
bruvzg
kobewi
Aleksey Vasenev
Hugo Locurcio
Fabio Alessandrelli
Alvin Wong
jsjtxietian
clayjohn
A Thousand Ships
K. S. Ernest (iFire) Lee
Aaron Franke
Danil Alexeev
Yorick de Wid
ajreckof
Jordyfel
Pylgos
Bo Thompson
ppphp
Paul Joannon
nfrmtk
Thaddeus Crews
Ryan
David Nikdel
Muller-Castro
vittorioromeo
scgm0
Matthew Edwards
Yuri Sizov
ACB
Juan Linietsky
RedMser
nklbdev
Wilson E. Alvarez
rsburke4
Saracen
Thomas ten Cate
Hein-Pieter van Braam
bitsawer
Micky
acazuc
Rindbee
Marcin Nowak
Ithamar R. Adema
Maxim Kulkin