CMapGeneratorWorker thread safety issue. Fixes #783.
VFS access moved to main thread, where random map scripts are preloaded and stored prior to use by the worker thread. Adds LoadGlobalScript to ScriptInterface, for evaluating script content in the global scope. This was SVN commit r9261.
This commit is contained in:
parent
225346ffc9
commit
481f570a0e
@ -19,10 +19,11 @@
|
||||
|
||||
#include "MapGenerator.h"
|
||||
|
||||
#include "lib/file/vfs/vfs_path.h"
|
||||
#include "lib/timer.h"
|
||||
#include "lib/utf8.h"
|
||||
#include "ps/CLogger.h"
|
||||
|
||||
|
||||
// TODO: what's a good default? perhaps based on map size
|
||||
#define RMS_RUNTIME_SIZE 96 * 1024 * 1024
|
||||
|
||||
@ -48,6 +49,9 @@ void CMapGeneratorWorker::Initialize(const VfsPath& scriptFile, const std::strin
|
||||
m_ScriptPath = scriptFile;
|
||||
m_Settings = settings;
|
||||
|
||||
// Preload random map and library scripts
|
||||
fs_util::ForEachFile(g_VFS, L"maps/random/", PreloadScript, (uintptr_t)this, L"*.js", fs_util::DIR_RECURSIVE);
|
||||
|
||||
// Launch the worker thread
|
||||
int ret = pthread_create(&m_WorkerThread, NULL, &RunThread, this);
|
||||
debug_assert(ret == 0);
|
||||
@ -96,7 +100,7 @@ bool CMapGeneratorWorker::Run()
|
||||
CScriptValRooted settingsVal = m_ScriptInterface->ParseJSON(m_Settings);
|
||||
if (settingsVal.undefined())
|
||||
{
|
||||
LOGERROR(L"CMapGeneratorWorker::Initialize: Failed to parse settings");
|
||||
LOGERROR(L"CMapGeneratorWorker::Run: Failed to parse settings");
|
||||
return false;
|
||||
}
|
||||
|
||||
@ -104,7 +108,7 @@ bool CMapGeneratorWorker::Run()
|
||||
uint32 seed;
|
||||
if (!m_ScriptInterface->GetProperty(settingsVal.get(), "Seed", seed))
|
||||
{ // No seed specified
|
||||
LOGWARNING(L"CMapGeneratorWorker::Initialize: No seed value specified - using 0");
|
||||
LOGWARNING(L"CMapGeneratorWorker::Run: No seed value specified - using 0");
|
||||
seed = 0;
|
||||
}
|
||||
|
||||
@ -113,19 +117,24 @@ bool CMapGeneratorWorker::Run()
|
||||
// Copy settings to global variable
|
||||
if (!m_ScriptInterface->SetProperty(m_ScriptInterface->GetGlobalObject(), "g_MapSettings", settingsVal))
|
||||
{
|
||||
LOGERROR(L"CMapGeneratorWorker::Initialize: Failed to define g_MapSettings");
|
||||
LOGERROR(L"CMapGeneratorWorker::Run: Failed to define g_MapSettings");
|
||||
return false;
|
||||
}
|
||||
|
||||
// Load RMS
|
||||
LOGMESSAGE(L"Loading RMS '%ls'", m_ScriptPath.string().c_str());
|
||||
if (!m_ScriptInterface->LoadGlobalScriptFile(m_ScriptPath))
|
||||
// Find RMS file and run
|
||||
ScriptFilesMap::iterator it = m_ScriptFiles.find(m_ScriptPath);
|
||||
if (it != m_ScriptFiles.end())
|
||||
{
|
||||
LOGERROR(L"CMapGeneratorWorker::Initialize: Failed to load RMS '%ls'", m_ScriptPath.string().c_str());
|
||||
return false;
|
||||
LOGMESSAGE(L"CMapGeneratorWorker::Run: Loading RMS '%ls'", m_ScriptPath.string().c_str());
|
||||
// Note: we're not really accessing the file here
|
||||
if (m_ScriptInterface->LoadGlobalScript(m_ScriptPath, it->second))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
LOGERROR(L"CMapGeneratorWorker::Run: Failed to load RMS '%ls'", m_ScriptPath.string().c_str());
|
||||
return false;
|
||||
}
|
||||
|
||||
int CMapGeneratorWorker::GetProgress()
|
||||
@ -183,38 +192,64 @@ bool CMapGeneratorWorker::LoadScripts(const std::wstring& libraryName)
|
||||
// Mark this as loaded, to prevent it recursively loading itself
|
||||
m_LoadedLibraries.insert(libraryName);
|
||||
|
||||
VfsPath path = L"maps/random/" + libraryName + L"/";
|
||||
VfsPaths pathnames;
|
||||
LOGMESSAGE(L"Loading '%ls' library", libraryName.c_str());
|
||||
|
||||
// Load all scripts in mapgen directory
|
||||
LibError ret = fs_util::GetPathnames(g_VFS, path, L"*.js", pathnames);
|
||||
if (ret == INFO::OK)
|
||||
std::wstring libraryPath = L"maps/random/" + libraryName + L"/";
|
||||
|
||||
// Iterate preloaded script map, running library scripts
|
||||
for (ScriptFilesMap::iterator it = m_ScriptFiles.begin(); it != m_ScriptFiles.end(); ++it)
|
||||
{
|
||||
for (VfsPaths::iterator it = pathnames.begin(); it != pathnames.end(); ++it)
|
||||
std::wstring path = it->first.string();
|
||||
if (path.find(libraryPath) == 0)
|
||||
{
|
||||
LOGMESSAGE(L"Loading map generator script '%ls'", it->string().c_str());
|
||||
|
||||
if (!m_ScriptInterface->LoadGlobalScriptFile(*it))
|
||||
// This script is part of the library, so load it
|
||||
// Note: we're not really accessing the file here
|
||||
if (m_ScriptInterface->LoadGlobalScript(path, it->second))
|
||||
{
|
||||
LOGERROR(L"Failed to load script '%ls'", it->string().c_str());
|
||||
LOGMESSAGE(L"Successfully loaded library script '%ls'", path.c_str());
|
||||
}
|
||||
else
|
||||
{
|
||||
// Script failed to run
|
||||
LOGERROR(L"Failed loading library script '%ls'", path.c_str());
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
// Some error reading directory
|
||||
wchar_t error[200];
|
||||
LOGERROR(L"Error reading scripts in directory '%ls': %hs", path.string().c_str(), error_description_r(ret, error, ARRAY_SIZE(error)));
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
LibError CMapGeneratorWorker::PreloadScript(const VfsPath& pathname, const FileInfo& UNUSED(fileInfo), const uintptr_t cbData)
|
||||
{
|
||||
CMapGeneratorWorker* self = (CMapGeneratorWorker*)cbData;
|
||||
|
||||
if (!VfsFileExists(g_VFS, pathname))
|
||||
{
|
||||
// This should never happen
|
||||
LOGERROR(L"CMapGeneratorWorker::PreloadScript: File '%ls' does not exist", pathname.string().c_str());
|
||||
return ERR::VFS_FILE_NOT_FOUND;
|
||||
}
|
||||
|
||||
CVFSFile file;
|
||||
|
||||
PSRETURN ret = file.Load(g_VFS, pathname);
|
||||
if (ret != PSRETURN_OK)
|
||||
{
|
||||
LOGERROR(L"CMapGeneratorWorker::PreloadScript: Failed to load file '%ls', error=%hs", pathname.string().c_str(), GetErrorString(ret));
|
||||
return ERR::FAIL;
|
||||
}
|
||||
|
||||
std::string content(file.GetBuffer(), file.GetBuffer() + file.GetBufferSize());
|
||||
|
||||
self->m_ScriptFiles.insert(std::make_pair(pathname, wstring_from_utf8(content)));
|
||||
return INFO::OK;
|
||||
}
|
||||
|
||||
//////////////////////////////////////////////////////////////////////////////////
|
||||
//////////////////////////////////////////////////////////////////////////////////
|
||||
|
||||
|
||||
CMapGenerator::CMapGenerator() : m_Worker(new CMapGeneratorWorker())
|
||||
{
|
||||
}
|
||||
|
@ -23,7 +23,9 @@
|
||||
#include "scriptinterface/ScriptInterface.h"
|
||||
|
||||
#include <boost/random/linear_congruential.hpp>
|
||||
#include <boost/unordered_map.hpp>
|
||||
|
||||
typedef boost::unordered_map<VfsPath, std::wstring> ScriptFilesMap;
|
||||
|
||||
class CMapGeneratorWorker;
|
||||
|
||||
@ -51,7 +53,7 @@ public:
|
||||
/**
|
||||
* Get status of the map generator thread
|
||||
*
|
||||
* @return Progress percentage 1-100 if active, 0 when finished, or -1 when
|
||||
* @return Progress percentage 1-100 if active, 0 when finished, or -1 on error
|
||||
*/
|
||||
int GetProgress();
|
||||
|
||||
@ -76,6 +78,8 @@ private:
|
||||
* - Initialize and constructor/destructor must be called from the main thread.
|
||||
* - ScriptInterface created and destroyed by thread
|
||||
* - StructuredClone used to return JS map data - jsvals can't be used across threads/runtimes
|
||||
* - VFS is not threadsafe, so preload all random map scripts in the main thread for later use
|
||||
* by the worker
|
||||
*/
|
||||
class CMapGeneratorWorker
|
||||
{
|
||||
@ -113,6 +117,7 @@ private:
|
||||
* Load all scripts of the given library
|
||||
*
|
||||
* @param libraryName String specifying name of the library (subfolder of ../maps/random/)
|
||||
* @return true if all scripts ran successfully, false if there's an error
|
||||
*/
|
||||
bool LoadScripts(const std::wstring& libraryName);
|
||||
|
||||
@ -126,9 +131,15 @@ private:
|
||||
shared_ptr<ScriptInterface::StructuredClone> m_MapData;
|
||||
boost::rand48 m_MapGenRNG;
|
||||
int m_Progress;
|
||||
ScriptInterface* m_ScriptInterface;
|
||||
VfsPath m_ScriptPath;
|
||||
ScriptInterface* m_ScriptInterface;
|
||||
std::string m_Settings;
|
||||
|
||||
// Since VFS is not threadsafe, use this map to store preloaded scripts
|
||||
ScriptFilesMap m_ScriptFiles;
|
||||
|
||||
// callback for VFS preloading
|
||||
static LibError PreloadScript(const VfsPath& pathname, const FileInfo& fileInfo, const uintptr_t cbData);
|
||||
|
||||
// Thread
|
||||
static void* RunThread(void* data);
|
||||
|
@ -790,6 +790,22 @@ bool ScriptInterface::LoadScript(const VfsPath& filename, const std::wstring& co
|
||||
return ok ? true : false;
|
||||
}
|
||||
|
||||
bool ScriptInterface::LoadGlobalScript(const VfsPath& filename, const std::wstring& code)
|
||||
{
|
||||
// Compile the code in strict mode, to encourage better coding practices and
|
||||
// to possibly help SpiderMonkey with optimisations
|
||||
std::wstring codeStrict = L"\"use strict\";\n" + code;
|
||||
utf16string codeUtf16(codeStrict.begin(), codeStrict.end());
|
||||
uintN lineNo = 0; // put the automatic 'use strict' on line 0, so the real code starts at line 1
|
||||
|
||||
jsval rval;
|
||||
JSBool ok = JS_EvaluateUCScript(m->m_cx, m->m_glob,
|
||||
reinterpret_cast<const jschar*> (codeUtf16.c_str()), (uintN)(codeUtf16.length()),
|
||||
utf8_from_wstring(filename.string()).c_str(), lineNo, &rval);
|
||||
|
||||
return ok ? true : false;
|
||||
}
|
||||
|
||||
bool ScriptInterface::LoadGlobalScriptFile(const VfsPath& path)
|
||||
{
|
||||
debug_assert(ThreadUtil::IsMainThread()); // VFS isn't thread-safe
|
||||
|
@ -237,6 +237,14 @@ public:
|
||||
*/
|
||||
bool LoadScript(const VfsPath& filename, const std::wstring& code);
|
||||
|
||||
/**
|
||||
* Load and execute the given script in the global scope.
|
||||
* @param filename Name for debugging purposes (not used to load the file)
|
||||
* @param code JS code to execute
|
||||
* @return true on successful compilation and execution; false otherwise
|
||||
*/
|
||||
bool LoadGlobalScript(const VfsPath& filename, const std::wstring& code);
|
||||
|
||||
/**
|
||||
* Load and execute the given script in the global scope.
|
||||
* @return true on successful compilation and execution; false otherwise
|
||||
@ -269,6 +277,9 @@ public:
|
||||
*/
|
||||
void DumpHeap();
|
||||
|
||||
/**
|
||||
* MaybeGC tries to determine whether garbage collection in cx's runtime would free up enough memory to be worth the amount of time it would take
|
||||
*/
|
||||
void MaybeGC();
|
||||
|
||||
/**
|
||||
|
Loading…
Reference in New Issue
Block a user