From 58dcf68c53efdc23a3f941ecccc0527c014d42a3 Mon Sep 17 00:00:00 2001 From: attila Date: Tue, 11 Feb 2025 11:24:20 +0100 Subject: [PATCH] Javascript: Fix eventual timeout when calling JSObject::invokeMethod repeatedly --- .../detail/juce_QuickJSHelpers.h | 27 +++++---- .../javascript/juce_JSObject.cpp | 10 ++-- .../javascript/juce_JSObject.h | 2 +- .../javascript/juce_JavascriptEngine.cpp | 57 ++++++++----------- 4 files changed, 48 insertions(+), 48 deletions(-) diff --git a/modules/juce_javascript/detail/juce_QuickJSHelpers.h b/modules/juce_javascript/detail/juce_QuickJSHelpers.h index 0246780e6d..d8226eb3b0 100644 --- a/modules/juce_javascript/detail/juce_QuickJSHelpers.h +++ b/modules/juce_javascript/detail/juce_QuickJSHelpers.h @@ -415,6 +415,12 @@ JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wsubobject-linkage") class QuickJSWrapper { public: + explicit QuickJSWrapper (const RelativeTime* maximumExecutionTimeIn) + : maximumExecutionTime { *maximumExecutionTimeIn } + { + qjs::JS_SetInterruptHandler (getQuickJSRuntime(), handleInterrupt, (void*) this); + } + qjs::JSContext* getQuickJSContext() const { return impl->context; @@ -425,27 +431,26 @@ public: return impl->runtime; } - /* Returning a value > 0 will interrupt the QuickJS engine. - */ - void setInterruptHandler (std::function interruptHandlerIn) + void resetTimeout() { - interruptHandler = std::move (interruptHandlerIn); - qjs::JS_SetInterruptHandler (getQuickJSRuntime(), handleInterrupt, (void*) this); + timeout = (int64) Time::getMillisecondCounterHiRes() + maximumExecutionTime.inMilliseconds(); + } + + void stop() + { + timeout = (int64) Time::getMillisecondCounterHiRes(); } private: static int handleInterrupt (qjs::JSRuntime*, void* opaque) { auto& self = *static_cast (opaque); - - if (self.interruptHandler != nullptr) - return self.interruptHandler(); - - return 0; + return (int64) Time::getMillisecondCounterHiRes() >= self.timeout; } std::unique_ptr impl = std::make_unique(); - std::function interruptHandler; + const RelativeTime& maximumExecutionTime; + std::atomic timeout{}; }; JUCE_END_IGNORE_WARNINGS_GCC_LIKE diff --git a/modules/juce_javascript/javascript/juce_JSObject.cpp b/modules/juce_javascript/javascript/juce_JSObject.cpp index f7fdd60591..33c6ad2eba 100644 --- a/modules/juce_javascript/javascript/juce_JSObject.cpp +++ b/modules/juce_javascript/javascript/juce_JSObject.cpp @@ -41,7 +41,7 @@ class JSObject::Impl public: using ValuePtr = detail::qjs::QuickJSContext::ValuePtr; - explicit Impl (const detail::QuickJSWrapper* engineIn) + explicit Impl (detail::QuickJSWrapper* engineIn) : Impl (engineIn, { detail::qjs::JS_GetGlobalObject (engineIn->getQuickJSContext()), engineIn->getQuickJSContext() }) { @@ -99,6 +99,8 @@ public: detail::VarOrError invokeMethod (const Identifier& methodName, Span args) const { + engine->resetTimeout(); + if (! hasProperty (methodName)) { jassertfalse; @@ -168,17 +170,17 @@ public: } private: - Impl (const detail::QuickJSWrapper* e, ValuePtr&& ptr) + Impl (detail::QuickJSWrapper* e, ValuePtr&& ptr) : engine (e), valuePtr (std::move (ptr)) { } - const detail::QuickJSWrapper* engine = nullptr; + detail::QuickJSWrapper* engine = nullptr; ValuePtr valuePtr; }; JUCE_END_IGNORE_WARNINGS_GCC_LIKE -JSObject::JSObject (const detail::QuickJSWrapper* engine) +JSObject::JSObject (detail::QuickJSWrapper* engine) : impl (new Impl (engine)) { } diff --git a/modules/juce_javascript/javascript/juce_JSObject.h b/modules/juce_javascript/javascript/juce_JSObject.h index ee606da926..3fa40221c1 100644 --- a/modules/juce_javascript/javascript/juce_JSObject.h +++ b/modules/juce_javascript/javascript/juce_JSObject.h @@ -57,7 +57,7 @@ public: To create a new JSObject pointing at the root object of the engine's context use JavascriptEngine::getRootObject(). */ - explicit JSObject (const detail::QuickJSWrapper* engine); + explicit JSObject (detail::QuickJSWrapper* engine); /** Destructor. */ ~JSObject(); diff --git a/modules/juce_javascript/javascript/juce_JavascriptEngine.cpp b/modules/juce_javascript/javascript/juce_JavascriptEngine.cpp index 1fff0d88d2..65205ce21d 100644 --- a/modules/juce_javascript/javascript/juce_JavascriptEngine.cpp +++ b/modules/juce_javascript/javascript/juce_JavascriptEngine.cpp @@ -42,22 +42,18 @@ public: using ValuePtr = detail::qjs::QuickJSContext::ValuePtr; //============================================================================== - Impl() + explicit Impl (const RelativeTime* maximumExecutionTimeIn) + : engine (std::make_unique (maximumExecutionTimeIn)) { - detail::DynamicObjectWrapper::createClass (engine.getQuickJSRuntime()); - - engine.setInterruptHandler ([this] - { - return (int64) Time::getMillisecondCounterHiRes() >= timeout; - }); + detail::DynamicObjectWrapper::createClass (engine->getQuickJSRuntime()); } void registerNativeObject (const Identifier& name, DynamicObject::Ptr dynamicObject, std::optional parent = std::nullopt) { - auto wrapper = std::make_unique (engine, dynamicObject); - auto* ctx = engine.getQuickJSContext(); + auto wrapper = std::make_unique (*engine, dynamicObject); + auto* ctx = engine->getQuickJSContext(); auto jsObject = JS_NewObjectClass (ctx, (int) detail::DynamicObjectWrapper::getClassId()); detail::qjs::JS_SetOpaque (jsObject, (void*) wrapper.get()); @@ -117,14 +113,19 @@ public: wrapper.release(); } - var evaluate (const String& code, Result* errorMessage, RelativeTime maxExecTime) + var evaluate (const String& code, Result* errorMessage) { - resetTimeout (maxExecTime); + engine->resetTimeout(); if (errorMessage != nullptr) *errorMessage = Result::ok(); - const auto result = detail::quickJSToJuce ({ JS_Eval (engine.getQuickJSContext(), code.toRawUTF8(), code.getNumBytesAsUTF8(), "", JS_EVAL_TYPE_GLOBAL), engine.getQuickJSContext() }); + const auto result = detail::quickJSToJuce ({ JS_Eval (engine->getQuickJSContext(), + code.toRawUTF8(), + code.getNumBytesAsUTF8(), + "", + JS_EVAL_TYPE_GLOBAL), + engine->getQuickJSContext() }); if (auto* v = std::get_if (&result)) return *v; @@ -136,21 +137,20 @@ public: return var::undefined(); } - Result execute (const String& code, RelativeTime maxExecTime) + Result execute (const String& code) { auto result = Result::ok(); - evaluate (code, &result, maxExecTime); + evaluate (code, &result); return result; } var callFunction (const Identifier& function, const var::NativeFunctionArgs& args, - Result* errorMessage, - RelativeTime maxExecTime) + Result* errorMessage) { - resetTimeout (maxExecTime); + engine->resetTimeout(); - auto* ctx = engine.getQuickJSContext(); + auto* ctx = engine->getQuickJSContext(); const auto functionStr = function.toString(); const auto fn = detail::qjs::JS_NewAtomLen (ctx, functionStr.toRawUTF8(), functionStr.getNumBytesAsUTF8()); @@ -179,29 +179,22 @@ public: void stop() noexcept { - timeout = (int64) Time::getMillisecondCounterHiRes(); + engine->stop(); } JSObject getRootObject() const { - return JSObject { &engine }; + return JSObject { engine.get() }; } private: - //============================================================================== - void resetTimeout (RelativeTime maxExecTime) - { - timeout = (int64) Time::getMillisecondCounterHiRes() + maxExecTime.inMilliseconds(); - } - - detail::QuickJSWrapper engine; - std::atomic timeout{}; + std::unique_ptr engine; }; //============================================================================== JavascriptEngine::JavascriptEngine() : maximumExecutionTime (15.0), - impl (std::make_unique()) + impl (std::make_unique (&maximumExecutionTime)) { } @@ -214,19 +207,19 @@ void JavascriptEngine::registerNativeObject (const Identifier& name, DynamicObje Result JavascriptEngine::execute (const String& javascriptCode) { - return impl->execute (javascriptCode, maximumExecutionTime); + return impl->execute (javascriptCode); } var JavascriptEngine::evaluate (const String& javascriptCode, Result* errorMessage) { - return impl->evaluate (javascriptCode, errorMessage, maximumExecutionTime); + return impl->evaluate (javascriptCode, errorMessage); } var JavascriptEngine::callFunction (const Identifier& function, const var::NativeFunctionArgs& args, Result* errorMessage) { - return impl->callFunction (function, args, errorMessage, maximumExecutionTime); + return impl->callFunction (function, args, errorMessage); } void JavascriptEngine::stop() noexcept