Skip to content

Fix race conditions in ArchiveFileSystem #313

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.

Sign up for GitHub

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

Open
seer-by-sentry wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix race conditions in ArchiveFileSystem #313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Core/GameEngine/Include/Common/ArchiveFileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "Common/AsciiString.h"
#include "Common/FileSystem.h" // for typedefs, etc.
#include "Common/STLTypedefs.h"
#include "mutex.h"

//----------------------------------------------------------------------------
// Forward References
Expand Down Expand Up @@ -169,6 +170,7 @@ class ArchiveFileSystem : public SubsystemInterface

ArchiveFileMap m_archiveFileMap;
ArchivedDirectoryInfo m_rootDirectory;
mutable FastCriticalSectionClass m_archiveDirectoryMutex; ///< Protects access to m_rootDirectory and all nested directory/file structures
};


Expand Down
7 changes: 7 additions & 0 deletions Core/GameEngine/Source/Common/System/ArchiveFileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ ArchiveFileSystem::~ArchiveFileSystem()

void ArchiveFileSystem::loadIntoDirectoryTree(ArchiveFile *archiveFile, Bool overwrite)
{
FastCriticalSectionClass::LockClass lock(m_archiveDirectoryMutex);

FilenameList filenameList;

Expand Down Expand Up @@ -240,6 +241,8 @@ void ArchiveFileSystem::loadMods()

Bool ArchiveFileSystem::doesFileExist(const Char *filename, FileInstance instance) const
{
FastCriticalSectionClass::LockClass lock(m_archiveDirectoryMutex);

ArchivedDirectoryInfoResult result = const_cast<ArchiveFileSystem*>(this)->getArchivedDirectoryInfo(filename);

if (!result.valid())
Expand All @@ -252,6 +255,8 @@ Bool ArchiveFileSystem::doesFileExist(const Char *filename, FileInstance instanc

ArchivedDirectoryInfo* ArchiveFileSystem::friend_getArchivedDirectoryInfo(const Char* directory)
{
FastCriticalSectionClass::LockClass lock(m_archiveDirectoryMutex);

ArchivedDirectoryInfoResult result = getArchivedDirectoryInfo(directory);

return result.dirInfo;
Expand Down Expand Up @@ -319,6 +324,8 @@ Bool ArchiveFileSystem::getFileInfo(const AsciiString& filename, FileInfo *fileI

ArchiveFile* ArchiveFileSystem::getArchiveFile(const AsciiString& filename, FileInstance instance) const
{
FastCriticalSectionClass::LockClass lock(m_archiveDirectoryMutex);

ArchivedDirectoryInfoResult result = const_cast<ArchiveFileSystem*>(this)->getArchivedDirectoryInfo(filename.str());

if (!result.valid())
Expand Down
4 changes: 4 additions & 0 deletions Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DFileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,9 +462,13 @@ void W3DFileSystem::reprioritizeTexturesBySize()
//
// Catered to specific game archives only. This ensures that user created archives are not included
// for the re-prioritization of textures.
//
// TheSuperHackers @bugfix 02/02/2025 Adds mutex locking to protect multimap access from race conditions
// during concurrent file existence checks and texture reprioritization.
//-------------------------------------------------------------------------------------------------
void W3DFileSystem::reprioritizeTexturesBySize(ArchivedDirectoryInfo& dirInfo)
{
// Note: This function is called with m_archiveDirectoryMutex already locked by friend_getArchivedDirectoryInfo
const char* const superiorArchive = "Textures.big";
const char* const inferiorArchive = "TexturesZH.big";

Expand Down
4 changes: 4 additions & 0 deletions GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DFileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,9 +489,13 @@ void W3DFileSystem::reprioritizeTexturesBySize()
//
// Catered to specific game archives only. This ensures that user created archives are not included
// for the re-prioritization of textures.
//
// TheSuperHackers @bugfix 02/02/2025 Adds mutex locking to protect multimap access from race conditions
// during concurrent file existence checks and texture reprioritization.
//-------------------------------------------------------------------------------------------------
void W3DFileSystem::reprioritizeTexturesBySize(ArchivedDirectoryInfo& dirInfo)
{
// Note: This function is called with m_archiveDirectoryMutex already locked by friend_getArchivedDirectoryInfo
const char* const superiorArchive = "Textures.big";
const char* const inferiorArchive = "TexturesZH.big";

Expand Down
Loading