From ec95bb93e10316b43895751fcec40a17c0e3438c Mon Sep 17 00:00:00 2001 From: "wrenczes@gmail.com" Date: Wed, 20 Apr 2011 09:03:08 +0000 Subject: [PATCH] Fix for a crash (I think the same one Zeth reported) where the game dies somewhere in file reading source - I wasn't paying close enough attention to the fact that there are in fact 3 separate caches, so each had their own mutex, so JFileSystem wasn't actually being protected from reentrancy. So, if the app tried to load an audio sample at the same time as an image, boom... --- projects/mtg/include/WResourceManagerImpl.h | 6 ------ projects/mtg/src/WResourceManager.cpp | 13 +++++++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/projects/mtg/include/WResourceManagerImpl.h b/projects/mtg/include/WResourceManagerImpl.h index 2447fcf4a..645ce19d3 100644 --- a/projects/mtg/include/WResourceManagerImpl.h +++ b/projects/mtg/include/WResourceManagerImpl.h @@ -4,7 +4,6 @@ #include "MTGDeck.h" #include "MTGCard.h" #include "utils.h" -#include "Threading.h" #include "WResourceManager.h" #include "WCachedResource.h" #include "WFont.h" @@ -120,11 +119,6 @@ protected: unsigned int cacheItems; int mError; - - // mutex meant for the cache map - boost::mutex mCacheMutex; - // mutex meant to protect against unthread-safe calls into JFileSystem, etc. - boost::mutex mLoadFunctionMutex; }; struct WManagedQuad diff --git a/projects/mtg/src/WResourceManager.cpp b/projects/mtg/src/WResourceManager.cpp index ecd82119a..bec6b618f 100644 --- a/projects/mtg/src/WResourceManager.cpp +++ b/projects/mtg/src/WResourceManager.cpp @@ -25,6 +25,11 @@ namespace const std::string kGenericCard("back.jpg"); const std::string kGenericThumbCard("back_thumb.jpg"); + + // mutex meant for the cache map + boost::mutex sCacheMutex; + // mutex meant to protect against unthread-safe calls into JFileSystem, etc. + boost::mutex sLoadFunctionMutex; } WResourceManager* WResourceManager::sInstance = NULL; @@ -1161,7 +1166,7 @@ cacheItem* WCache::Get(int id, const string& filename, i //Not managed, so look in cache. if (style != RETRIEVE_MANAGE) { - boost::mutex::scoped_lock lock(mCacheMutex); + boost::mutex::scoped_lock lock(sCacheMutex); //DebugTrace("Cache lock acquired, looking up index " << lookup); it = cache.find(lookup); //Well, we've found something... @@ -1216,7 +1221,7 @@ cacheItem* WCache::LoadIntoCache(int id, const string& f // the shared cache container, and this separate lock was added to insulate us against thread safety issues in JGE. In particular, // JFileSystem is particularly unsafe, as it assumes that we have only one zip loaded at a time... rather than add mutexes in JGE, // I've kept it local to here. - boost::mutex::scoped_lock functionLock(mLoadFunctionMutex); + boost::mutex::scoped_lock functionLock(sLoadFunctionMutex); cacheItem* item = AttemptNew(filename, submode); //assert(item); if (style == RETRIEVE_MANAGE) @@ -1234,7 +1239,7 @@ cacheItem* WCache::LoadIntoCache(int id, const string& f { if (mError == CACHE_ERROR_404 || item) { - boost::mutex::scoped_lock lock(mCacheMutex); + boost::mutex::scoped_lock lock(sCacheMutex); cache[id] = item; DebugTrace("inserted item ptr " << ToHex(item) << " at index " << id); } @@ -1331,7 +1336,7 @@ bool WCache::Cleanup() // this looks redundant, but the idea is, don't grab the mutex if there's no work to do if (RequiresOldItemCleanup()) { - boost::mutex::scoped_lock lock(mCacheMutex); + boost::mutex::scoped_lock lock(sCacheMutex); while (RequiresOldItemCleanup()) { if (!RemoveOldest())