[STFS] Add critical section to file reading #2123
Conversation
Seems like reading same file concurrently from many threads can cause situation when random thread receives incorrect data
| auto& file = entry_->files()->at(record.file); | ||
| xe::filesystem::Seek(file, record.offset + read_offset, SEEK_SET); | ||
| auto num_read = fread(p, 1, read_length, file); | ||
| auto global_lock = global_critical_region_.Acquire(); |
There was a problem hiding this comment.
Is that a race between guess threads accessing the same guest file handle, or different guest handles implemented through one host file handle? If that's the latter, we should be opening the host container file multiple times, for each guest handle, or if possible, use a memory mapping. Using the global critical region here will create a lot of contention that may block other threads for milliseconds — the comment in xenia/base/mutex.h explicitly says that no I/O must be done there.
There was a problem hiding this comment.
Ohh my bad for not reading comments in mutex.h.
It uses multiple handles, so rewrites then to allow memory mapping probably (if possible)
There was a problem hiding this comment.
That's strange, I thought it was already using memory mapping for STFS? Maybe I'm forgetting when it got changed. I know I wanted the capability to do both, since we need the ability to load STFS packages nested within a non-host filesystem (ie, such as another STFS package).
Affected titles:
xenia-project/game-compatibility#827
xenia-project/game-compatibility#125
Description:
These titles use multiple threads to load same file, which in case of extracted file doesn't make any issue, however for STFS package it is huge problem as it can randomly receive incorrect data.
I checked it multiple times and calculated hashes of loaded data between extracted and packed game.
Conclusion for me is simple, at certain point thread starts getting invalid data, which is simply resolved by including critical section