Nabla
Nabla copied to clipboard
Add thread safe mutability for IAsset and eliminate a possible "double load" bug
Describe the bug
Prevent Double Loads of assets caused by multiple threads deciding to load an asset because all have queried the asset cache before either was done loading
Description of the related problem
Thread Safety
Assets should have a lock which should prevent read-accesses while an asset is being modified, and more than one thread/fiber modifying an asset.
Double Asset Loads
There is a possibility of a double load of an asset occuring if following events occur in this order:
- Thread A checks our thread-safe asset cache for path "huge.obj"
- Thread A decides to load the OBJ
- Thread B checks the asset cache for the same path
- Thread B decides to load the OBJ
- Thread A inserts loaded OBJ into cache
- Thread B inserts loaded OBJ into cache
Because the Cache is a multimap, both assets which are identical sit in the cache. This order of events is not the only one that produces the bug, other orders such as 123465, 123546, 132456, etc. will result in this problem.
Solution proposal
Thread Safety
Improve the Readers-Writers Lock in FW_Mutex to be more in line with std::recursive_lock APIs, at least the reading lock should be able to be recursive (writer lock not so much).
There should be an explicit method to:
- acquire/release a reader lock
- acquire/release a writer lock
- upgrade/downgrade an already acquired reader lock to/from a writer lock without getting preempted
If writer lock has not been acquired then all the mutators of IAsset should assert.
However const-qualified functions should acquire the reader lock implicitly.
Double Asset Loads
We need to improve our cache container to be able to take "reservations" for keys, that is:
- If no concrete cached value or reservation has been made, proceed 1a. Reserve a key value and place a dummy value indicating key has been reserved 1b. Load the asset 1c. Swap the reservation for the loaded asset
- If reservation has been already made, then wait until the asset has been loaded and reservation changes
- If concrete value is present then just return the asset
This might require a readers/writers lock, and/or sempahores/condition variables.
We need to analyze this design for the possibility of dead-locks because assets are DAGs and during the process of loading a single thread could put down multiple reservations.
a reader-parallel hashmap / CObjectCache would be useful too
https://github.com/buildaworldnet/IrrlichtBAW/issues/364