diff --git a/code/framework/src/scripting/engines/node/engine.cpp b/code/framework/src/scripting/engines/node/engine.cpp index ea9fd0f74..88e3bba04 100644 --- a/code/framework/src/scripting/engines/node/engine.cpp +++ b/code/framework/src/scripting/engines/node/engine.cpp @@ -20,13 +20,16 @@ require("vm").runInThisContext(process.argv[1]); namespace Framework::Scripting::Engines::Node { EngineError Engine::Init(SDKRegisterCallback cb) { + if (_wasNodeAlreadyInited) { + return EngineError::ENGINE_NODE_INIT_FAILED; + } // Define the arguments to be passed to the node instance std::vector args = {"mafiahub-server", "--experimental-specifier-resolution=node", "--no-warnings"}; std::vector exec_args {}; std::vector errors {}; // Initialize the node with the provided arguments - int initCode = node::InitializeNodeWithArgs(&args, &exec_args, &errors); + int initCode = node::InitializeNodeWithArgs(&args, &exec_args, &errors); if (initCode != 0) { for (std::string &error : errors) { Logging::GetLogger(FRAMEWORK_INNER_SCRIPTING)->debug("Failed to initialize node: {}", error); @@ -36,11 +39,13 @@ namespace Framework::Scripting::Engines::Node { // Initialize the platform _platform = node::MultiIsolatePlatform::Create(4); - v8::V8::InitializePlatform(_platform.get()); + v8::V8::InitializePlatform(Engine::_platform.get()); v8::V8::Initialize(); + uv_loop_init(&uv_loop); + // Allocate and register the isolate - _isolate = node::NewIsolate(node::CreateArrayBufferAllocator(), uv_default_loop(), _platform.get()); + _isolate = node::NewIsolate(node::CreateArrayBufferAllocator(), &uv_loop, _platform.get()); // Allocate our scopes v8::Locker locker(_isolate); @@ -58,13 +63,17 @@ namespace Framework::Scripting::Engines::Node { // Reset and save the global object template pointer _globalObjectTemplate.Reset(_isolate, global); - // Ye Logging::GetLogger(FRAMEWORK_INNER_SCRIPTING)->debug("Node.JS engine initialized!"); _isShuttingDown = false; + _wasNodeAlreadyInited = true; return EngineError::ENGINE_NONE; } EngineError Engine::Shutdown() { + if (!_wasNodeAlreadyInited) { + return EngineError::ENGINE_NODE_SHUTDOWN_FAILED; + } + if (!_platform) { return EngineError::ENGINE_PLATFORM_NULL; } @@ -75,23 +84,31 @@ namespace Framework::Scripting::Engines::Node { _isShuttingDown = true; - v8::SealHandleScope seal(_isolate); - // Drain the remaining tasks while there are available ones - do { - uv_run(uv_default_loop(), UV_RUN_DEFAULT); - _platform->DrainTasks(_isolate); - } while (uv_loop_alive(uv_default_loop())); - - // Unregister the isolate from the platform + // Release the isolate + bool platform_finished = false; + _platform->AddIsolateFinishedCallback( + _isolate, + [](void *data) { + *static_cast(data) = true; + }, + &platform_finished); _platform->UnregisterIsolate(_isolate); + + // Wait until the platform has cleaned up all relevant resources. + { + v8::Locker locker(_isolate); + v8::Isolate::Scope isolate_scope(_isolate); + while (!platform_finished) uv_run(&uv_loop, UV_RUN_ONCE); + uv_loop_close(&uv_loop); + } - // Release the isolate _isolate->Dispose(); + _isolate = nullptr; // Release node and v8 engines - node::FreePlatform(_platform.release()); v8::V8::Dispose(); - v8::V8::ShutdownPlatform(); + v8::V8::DisposePlatform(); + node::FreePlatform(_platform.release()); return EngineError::ENGINE_NONE; } @@ -110,21 +127,29 @@ namespace Framework::Scripting::Engines::Node { } // Update the base scripting layer - v8::Locker locker(_isolate); - v8::Isolate::Scope isolateScope(_isolate); - v8::SealHandleScope seal(_isolate); - _platform->DrainTasks(_isolate); + { + v8::Locker locker(_isolate); + v8::Isolate::Scope isolateScope(_isolate); + v8::SealHandleScope seal(_isolate); + _platform->DrainTasks(_isolate); + + // Notify the gamemode, if loaded + if (_gamemodeLoaded) { + InvokeEvent(Events[EventIDs::GAMEMODE_UPDATED]); + } + } // Update the file watcher - if (!_isShuttingDown && Utils::Time::Compare(_nextFileWatchUpdate, Utils::Time::GetTimePoint()) < 0) { + if (_watcher && !_isShuttingDown && Utils::Time::Compare(_nextFileWatchUpdate, Utils::Time::GetTimePoint()) < 0) { // Process the file changes watcher - _watcher.watch(0); + _watcher->watch(0); _nextFileWatchUpdate = Utils::Time::Add(Utils::Time::GetTimePoint(), _fileWatchUpdatePeriod); - } - // Notify the gamemode, if loaded - if(_gamemodeLoaded){ - InvokeEvent(Events[EventIDs::GAMEMODE_UPDATED]); + if (_shouldReloadWatcher) { + _shouldReloadWatcher = false; + delete _watcher; + PreloadGamemode(_gamemodePath); + } } } @@ -152,6 +177,8 @@ namespace Framework::Scripting::Engines::Node { return false; } + _gamemodePath = mainPath; + auto root = nlohmann::json::parse(packageFileContent); try { _gamemodeMetadata.name = root["name"].get(); @@ -216,9 +243,9 @@ namespace Framework::Scripting::Engines::Node { // Create the execution environment node::EnvironmentFlags::Flags flags = (node::EnvironmentFlags::Flags)(node::EnvironmentFlags::kOwnsProcessState); - _gamemodeData = node::CreateIsolateData(_isolate, uv_default_loop(), GetPlatform()); + _gamemodeData = node::CreateIsolateData(_isolate, &uv_loop, GetPlatform()); std::vector argv = {"mafiahub-gamemode"}; - _gamemodeEnvironment = node::CreateEnvironment(_gamemodeData, context, argv, argv, flags); + _gamemodeEnvironment = node::CreateEnvironment(_gamemodeData, context, argv, argv, flags); // Make sure isolate is linked to our node environment node::IsolateSettings is; @@ -240,30 +267,46 @@ namespace Framework::Scripting::Engines::Node { bool Engine::UnloadGamemode(std::string mainPath) { // If gamemode is not loaded, don't unload it - if(!_gamemodeLoaded) { + if (!_gamemodeLoaded) { return false; } - // Scope the gamemode - v8::Locker locker(_isolate); - v8::Isolate::Scope isolateScope(_isolate); - v8::HandleScope handleScope(_isolate); - v8::Context::Scope scope(GetContext()); + // Stop node environment + node::Stop(_gamemodeEnvironment); - // Exit node environment - node::EmitProcessBeforeExit(_gamemodeEnvironment); - node::EmitProcessExit(_gamemodeEnvironment); + { + // Scope the gamemode + v8::Locker locker(_isolate); + v8::Isolate::Scope isolate_scope(_isolate); + { + v8::SealHandleScope seal(_isolate); - // Unregister the SDK - // TODO: fix me + bool more = false; + // Drain the remaining tasks while there are available ones + do { + uv_run(&uv_loop, UV_RUN_DEFAULT); + _platform->DrainTasks(_isolate); - // Stop node environment - node::Stop(_gamemodeEnvironment); + more = uv_loop_alive(&uv_loop); + if (more) + continue; + + if (node::EmitProcessBeforeExit(_gamemodeEnvironment).IsNothing()) + break; + + more = uv_loop_alive(&uv_loop); + } while (more); - // Release memory - node::FreeEnvironment(_gamemodeEnvironment); - node::FreeIsolateData(_gamemodeData); + // Exit node environment + node::EmitProcessExit(_gamemodeEnvironment); + } + + // Release memory + node::FreeIsolateData(_gamemodeData); + node::FreeEnvironment(_gamemodeEnvironment); + } + _gamemodeEventHandlers.clear(); _gamemodeLoaded = false; return true; } @@ -320,13 +363,18 @@ namespace Framework::Scripting::Engines::Node { return false; } + _watcher = new cppfs::FileWatcher; + Logging::GetLogger(FRAMEWORK_INNER_SCRIPTING)->debug("Watching '{}' changes", dir.path().c_str()); - _watcher.add(dir, cppfs::FileCreated | cppfs::FileRemoved | cppfs::FileModified | cppfs::FileAttrChanged, cppfs::Recursive); - _watcher.addHandler([this, path](cppfs::FileHandle &fh, cppfs::FileEvent ev) { + _watcher->add(dir, cppfs::FileCreated | cppfs::FileRemoved | cppfs::FileModified | cppfs::FileAttrChanged, cppfs::Recursive); + _watcher->addHandler([this, path](cppfs::FileHandle &fh, cppfs::FileEvent ev) { + if (_shouldReloadWatcher) + return; + Logging::GetLogger(FRAMEWORK_INNER_SCRIPTING)->debug("Gamemode is reloaded due to the file changes"); // Close the gamemode first, we'll start with a clean slate if(this->IsGamemodeLoaded() && UnloadGamemode(path)){ - LoadGamemode(path); + _shouldReloadWatcher = true; } }); diff --git a/code/framework/src/scripting/engines/node/engine.h b/code/framework/src/scripting/engines/node/engine.h index 0ed18898c..a18e242a3 100644 --- a/code/framework/src/scripting/engines/node/engine.h +++ b/code/framework/src/scripting/engines/node/engine.h @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -24,14 +25,14 @@ #include "../callback.h" #include "../engine.h" +#include "sdk.h" #include "v8_helpers/v8_string.h" #include "v8_helpers/v8_try_catch.h" -#include "sdk.h" -#define V8_RESOURCE_LOCK(engine)\ - v8::Locker locker(engine->GetIsolate());\ - v8::Isolate::Scope isolateScope(engine->GetIsolate());\ - v8::HandleScope handleScope(engine->GetIsolate());\ +#define V8_RESOURCE_LOCK(engine) \ + v8::Locker locker(engine->GetIsolate()); \ + v8::Isolate::Scope isolateScope(engine->GetIsolate()); \ + v8::HandleScope handleScope(engine->GetIsolate()); \ v8::Context::Scope contextScope(engine->GetContext()); namespace Framework::Scripting::Engines::Node { @@ -48,25 +49,29 @@ namespace Framework::Scripting::Engines::Node { SDK *_sdk = nullptr; // Global - cppfs::FileWatcher _watcher; + cppfs::FileWatcher *_watcher; Utils::Time::TimePoint _nextFileWatchUpdate; int32_t _fileWatchUpdatePeriod = 1000; // Global engine + static inline bool _wasNodeAlreadyInited = false; v8::Isolate *_isolate; v8::Persistent _globalObjectTemplate; std::unique_ptr _platform; v8::Persistent _context; std::string _modName; std::atomic _isShuttingDown = false; + uv_loop_t uv_loop; // Gamemode - std::atomic _gamemodeLoaded = false; + std::atomic _gamemodeLoaded = false; + std::string _gamemodePath; GamemodeMetadata _gamemodeMetadata = {}; v8::Persistent _gamemodeScript; - node::IsolateData *_gamemodeData = nullptr; - node::Environment *_gamemodeEnvironment = nullptr; - + node::IsolateData *_gamemodeData = nullptr; + node::Environment *_gamemodeEnvironment = nullptr; + bool _shouldReloadWatcher = false; + public: std::map> _gamemodeEventHandlers; @@ -85,19 +90,19 @@ namespace Framework::Scripting::Engines::Node { bool WatchGamemodeChanges(std::string); template - void InvokeEvent(const std::string name, Args ...args) { + void InvokeEvent(const std::string name, Args... args) { v8::Locker locker(GetIsolate()); v8::Isolate::Scope isolateScope(GetIsolate()); v8::HandleScope handleScope(GetIsolate()); v8::Context::Scope contextScope(_context.Get(_isolate)); - //const auto eventName = Helpers::MakeString(_isolate, name); + // const auto eventName = Helpers::MakeString(_isolate, name); if (_gamemodeEventHandlers[name].empty()) { return; } - constexpr int const arg_count = sizeof...(Args); + constexpr int const arg_count = sizeof...(Args); v8::Local v8_args[arg_count ? arg_count : 1] = {v8pp::to_v8(_isolate, std::forward(args))...}; for (auto it = _gamemodeEventHandlers[name].begin(); it != _gamemodeEventHandlers[name].end(); ++it) { @@ -106,9 +111,9 @@ namespace Framework::Scripting::Engines::Node { it->Get(_isolate)->Call(_context.Get(_isolate), v8::Undefined(_isolate), arg_count, v8_args); if (tryCatch.HasCaught()) { - auto context = _context.Get(_isolate); - v8::Local message = tryCatch.Message(); - v8::Local exception = tryCatch.Exception(); + auto context = _context.Get(_isolate); + v8::Local message = tryCatch.Message(); + v8::Local exception = tryCatch.Exception(); v8::MaybeLocal maybeSourceLine = message->GetSourceLine(context); v8::Maybe line = message->GetLineNumber(context); v8::ScriptOrigin origin = message->GetScriptOrigin(); diff --git a/code/framework/src/scripting/errors.h b/code/framework/src/scripting/errors.h index 339d69d15..6d22574df 100644 --- a/code/framework/src/scripting/errors.h +++ b/code/framework/src/scripting/errors.h @@ -9,7 +9,7 @@ #pragma once namespace Framework::Scripting { - enum class EngineError { ENGINE_NONE, ENGINE_NODE_INIT_FAILED, ENGINE_PLATFORM_INIT_FAILED, ENGINE_V8_INIT_FAILED, ENGINE_UV_LOOP_INIT_FAILED, ENGINE_ISOLATE_ALLOCATION_FAILED, ENGINE_ISOLATE_DATA_ALLOCATION_FAILED, ENGINE_PLATFORM_NULL, ENGINE_ISOLATE_NULL }; + enum class EngineError { ENGINE_NONE, ENGINE_NODE_INIT_FAILED, ENGINE_PLATFORM_INIT_FAILED, ENGINE_V8_INIT_FAILED, ENGINE_UV_LOOP_INIT_FAILED, ENGINE_ISOLATE_ALLOCATION_FAILED, ENGINE_ISOLATE_DATA_ALLOCATION_FAILED, ENGINE_PLATFORM_NULL, ENGINE_ISOLATE_NULL, ENGINE_NODE_SHUTDOWN_FAILED }; enum class ModuleError { MODULE_NONE, MODULE_ENGINE_NULL, MODULE_ENGINE_INIT_FAILED, MODULE_RESOURCE_MANAGER_NULL }; diff --git a/code/tests/modules/scripting_engine_ut.h b/code/tests/modules/scripting_engine_ut.h index 6ebd15ba3..165bd48e4 100644 --- a/code/tests/modules/scripting_engine_ut.h +++ b/code/tests/modules/scripting_engine_ut.h @@ -14,7 +14,7 @@ MODULE(scripting_engine, { using namespace Framework::Scripting; - IT("can allocate and deallocate a valid scripting engine instance, then do it again to test re-entry", { + IT("can allocate and deallocate a valid scripting engine instance", { Module *pEngine = new Module; // Init the engine and make sure everything went fine @@ -26,23 +26,6 @@ MODULE(scripting_engine, { // Shutdown the engine and make sure everything went down EQUALS(pEngine->Shutdown(), ModuleError::MODULE_NONE); EQUALS(reinterpret_cast(pEngine->GetEngine())->GetIsolate(), nullptr); - EQUALS(reinterpret_cast(pEngine->GetEngine())->GetPlatform(), nullptr); - - delete pEngine; - - // Intentionally duplicated to test engine for re-entry - pEngine = new Module; - - // Init the engine and make sure everything went fine - EQUALS(pEngine->Init(EngineTypes::ENGINE_NODE, NULL), ModuleError::MODULE_NONE); - NEQUALS(pEngine->GetEngine(), nullptr); - NEQUALS(reinterpret_cast(pEngine->GetEngine())->GetIsolate(), nullptr); - NEQUALS(reinterpret_cast(pEngine->GetEngine())->GetPlatform(), nullptr); - - // Shutdown the engine and make sure everything went down - EQUALS(pEngine->Shutdown(), ModuleError::MODULE_NONE); - EQUALS(reinterpret_cast(pEngine->GetEngine())->GetIsolate(), nullptr); - EQUALS(reinterpret_cast(pEngine->GetEngine())->GetPlatform(), nullptr); delete pEngine; });