diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c50bfa07..9f2b2c3b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -431,7 +431,7 @@ For now at least, we instead use raw pointers for this case. We still don't fully trust Python with the lifecyce of `ValueHandle` pointers; when Python passes these pointers back to C++, we still check validity by looking up the pointer as a key into a map (which then lets the C++ side of PyMiniRacer find the _rest_ -of the `Value` object). The C++ `MiniRacer::ValueFactory` can authoritatively destruct +of the `Value` object). The C++ `MiniRacer::ValueRegistry` can authoritatively destruct any dangling `Value` objects when it exits. This last especially helps with an odd scenario introduced by Python `__del__`: the diff --git a/README.md b/README.md index 794d648d..6fa3fc4e 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,14 @@ respectively: [None, JSUndefined] ``` +JavaScript strings are represented as `JSString`, a subclass of `str` which enables them +to be efficiently passed back to JavaScript: + +```python + >>> from py_mini_racer import JSString + >>> assert isinstance(ctx.eval("'foo'"), JSString) +``` + You can prevent runaway execution in synchronous code using the `timeout_sec` parameter: ```python diff --git a/src/py_mini_racer/__init__.py b/src/py_mini_racer/__init__.py index 68cc0ef8..d2f4d55f 100644 --- a/src/py_mini_racer/__init__.py +++ b/src/py_mini_racer/__init__.py @@ -24,6 +24,7 @@ JSMappedObject, JSObject, JSPromise, + JSString, JSSymbol, JSUndefined, JSUndefinedType, @@ -45,6 +46,7 @@ "JSParseException", "JSPromise", "JSPromiseError", + "JSString", "JSSymbol", "JSTimeoutException", "JSUndefined", diff --git a/src/py_mini_racer/_context.py b/src/py_mini_racer/_context.py index 36515de7..4215ddab 100644 --- a/src/py_mini_racer/_context.py +++ b/src/py_mini_racer/_context.py @@ -8,7 +8,7 @@ from typing import TYPE_CHECKING, Any, NewType, Protocol, cast from py_mini_racer._dll import init_mini_racer -from py_mini_racer._exc import JSEvalException, JSPromiseError +from py_mini_racer._exc import JSPromiseError from py_mini_racer._types import ( CancelableJSFunction, JSArray, @@ -99,7 +99,7 @@ class Context: _active_cancelable_mr_task_callbacks: dict[int, Callable[[ValueHandle], None]] = ( field(default_factory=dict) ) - _non_cancelable_mr_task_results_queue: queue.Queue[ValueHandle] = field( + _non_cancelable_mr_task_results_queue: queue.Queue[RawValueHandleType] = field( default_factory=queue.Queue ) @@ -118,21 +118,19 @@ def handle_callback_from_v8( # All work on the Isolate is blocked until this callback returns. That may # may in turn be blocking incoming calls from Python, including other threads, # asyncio event loops, etc. So we need to get out fast! - # We limit ourselves to wrapping the incoming handle (so we don't leak memory) - # and enqueing the incoming work for decoupled processing. - - val_handle = self._wrap_raw_handle(raw_val_handle) + # Just shove the handle onto a queue for decoupled processing and get out. if callback_id == _UNCANCELABLE_TASK_CALLBACK_ID: - self._non_cancelable_mr_task_results_queue.put(val_handle) + self._non_cancelable_mr_task_results_queue.put(raw_val_handle) else: self.event_loop.call_soon_threadsafe( - self._handle_callback_from_v8_on_event_loop, callback_id, val_handle + self._handle_callback_from_v8_on_event_loop, callback_id, raw_val_handle ) def _handle_callback_from_v8_on_event_loop( - self, callback_id: int, val_handle: ValueHandle + self, callback_id: int, raw_val_handle: RawValueHandleType ) -> None: + val_handle = self._wrap_raw_handle(raw_val_handle) try: callback = self._active_cancelable_mr_task_callbacks[callback_id] except KeyError: @@ -508,7 +506,7 @@ def callback(val_handle: ValueHandle) -> None: try: value = self._value_handle_to_python(val_handle) - except JSEvalException as e: + except Exception as e: # noqa: BLE001 future.set_exception(e) return @@ -538,7 +536,8 @@ def _run_uncancelable_mr_task( assert self.are_we_running_on_the_mini_racer_event_loop() _task_id = dll_method(self._ctx, *args, _UNCANCELABLE_TASK_CALLBACK_ID) - val_handle = self._non_cancelable_mr_task_results_queue.get() + raw_val_handle = self._non_cancelable_mr_task_results_queue.get() + val_handle = self._wrap_raw_handle(raw_val_handle) return self._value_handle_to_python(val_handle) def _value_handle_to_python( diff --git a/src/py_mini_racer/_objects.py b/src/py_mini_racer/_objects.py index 64ee64c9..cd61e2ea 100644 --- a/src/py_mini_racer/_objects.py +++ b/src/py_mini_racer/_objects.py @@ -26,6 +26,7 @@ JSMappedObject, JSObject, JSPromise, + JSString, JSSymbol, JSUndefined, JSUndefinedType, @@ -35,6 +36,8 @@ if TYPE_CHECKING: from collections.abc import Generator, Iterator, Sequence + from typing_extensions import Self + from py_mini_racer._context import Context from py_mini_racer._dll import RawValueHandleTypeImpl from py_mini_racer._value_handle import ValueHandle @@ -192,6 +195,16 @@ def __await__(self) -> Generator[Any, None, Any]: return self._ctx.await_promise(self).__await__() +class JSStringImpl(JSObjectImpl, JSString): + def __new__(cls, ctx: Context, handle: ValueHandle, content: str) -> Self: + del ctx, handle + return super().__new__(cls, content) + + def __init__(self, ctx: Context, handle: ValueHandle, content: str) -> None: + del content + JSObjectImpl.__init__(self, ctx, handle) + + class _ArrayBufferByte(ctypes.Structure): # Cannot use c_ubyte directly because it uses -#include #include "value.h" namespace MiniRacer { using RawCallback = void (*)(uint64_t, ValueHandle*); -using CallbackFn = std::function; - } // end namespace MiniRacer #endif // INCLUDE_MINI_RACER_CALLBACK_H diff --git a/src/v8_py_frontend/cancelable_task_runner.h b/src/v8_py_frontend/cancelable_task_runner.h index 25755628..a19083ab 100644 --- a/src/v8_py_frontend/cancelable_task_runner.h +++ b/src/v8_py_frontend/cancelable_task_runner.h @@ -8,7 +8,6 @@ #include #include "id_maker.h" #include "isolate_manager.h" -#include "v8-isolate.h" namespace MiniRacer { @@ -78,7 +77,7 @@ class CancelableTask : public CancelableTaskBase { OnCompleted on_completed, OnCanceled on_canceled); - void Run(v8::Isolate* isolate); + void Run(); void Cancel(IsolateManager* isolate_manager) override; private: @@ -108,10 +107,8 @@ inline auto CancelableTaskManager::Schedule(Runnable runnable, const uint64_t task_id = task_id_holder.GetId(); - std::future fut = isolate_manager_->Run( - [holder = std::move(task_id_holder), task](v8::Isolate* isolate) mutable { - task->Run(isolate); - }); + std::future fut = isolate_manager_->Schedule( + [holder = std::move(task_id_holder), task]() mutable { task->Run(); }); task->SetFuture(std::move(fut)); @@ -129,8 +126,7 @@ inline CancelableTask::CancelableTask( state_(State::kNotStarted) {} template -inline void CancelableTask::Run( - v8::Isolate* isolate) { +inline void CancelableTask::Run() { bool was_canceled_before_run = false; { const std::lock_guard lock(mutex_); @@ -146,7 +142,7 @@ inline void CancelableTask::Run( return; } - auto result = runnable_(isolate); + auto result = runnable_(); bool was_canceled_during_run = false; { diff --git a/src/v8_py_frontend/code_evaluator.cc b/src/v8_py_frontend/code_evaluator.cc index d1f3c2e5..4b945e92 100644 --- a/src/v8_py_frontend/code_evaluator.cc +++ b/src/v8_py_frontend/code_evaluator.cc @@ -9,32 +9,29 @@ #include #include #include -#include "context_holder.h" +#include "isolate_manager.h" #include "isolate_memory_monitor.h" #include "value.h" namespace MiniRacer { -CodeEvaluator::CodeEvaluator(ContextHolder* context, +CodeEvaluator::CodeEvaluator(IsolateManager* isolate_manager, ValueFactory* val_factory, IsolateMemoryMonitor* memory_monitor) - : context_(context), + : isolate_manager_(isolate_manager), val_factory_(val_factory), memory_monitor_(memory_monitor) {} -auto CodeEvaluator::Eval(v8::Isolate* isolate, Value* code_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(context); +auto CodeEvaluator::Eval(Value* code_ptr) -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); const v8::TryCatch trycatch(isolate); - const v8::Local local_code_val = - code_ptr->ToV8Value(isolate, context); + const v8::Local local_code_val = code_ptr->Global()->Get(isolate); if (!local_code_val->IsString()) { - return val_factory_->New("code is not a string", type_execute_exception); + return val_factory_->NewFromString("code is not a string", + type_execute_exception); } const v8::Local local_code_str = local_code_val.As(); @@ -43,22 +40,24 @@ auto CodeEvaluator::Eval(v8::Isolate* isolate, Value* code_ptr) -> Value::Ptr { v8::ScriptOrigin script_origin( v8::String::NewFromUtf8Literal(isolate, "")); + const v8::Local context = isolate_manager_->GetLocalContext(); + v8::Local script; if (!v8::Script::Compile(context, local_code_str, &script_origin) .ToLocal(&script) || script.IsEmpty()) { - return val_factory_->New(context, trycatch.Message(), trycatch.Exception(), - type_parse_exception); + return val_factory_->NewFromException( + trycatch.Message(), trycatch.Exception(), type_parse_exception); } v8::MaybeLocal maybe_value = script->Run(context); if (!maybe_value.IsEmpty()) { - return val_factory_->New(context, maybe_value.ToLocalChecked()); + return val_factory_->NewFromAny(maybe_value.ToLocalChecked()); } // Didn't execute. Find an error: if (memory_monitor_->IsHardMemoryLimitReached()) { - return val_factory_->New("", type_oom_exception); + return val_factory_->NewFromString("", type_oom_exception); } ValueTypes result_type = type_execute_exception; @@ -66,8 +65,8 @@ auto CodeEvaluator::Eval(v8::Isolate* isolate, Value* code_ptr) -> Value::Ptr { result_type = type_terminated_exception; } - return val_factory_->New(context, trycatch.Message(), trycatch.Exception(), - result_type); + return val_factory_->NewFromException(trycatch.Message(), + trycatch.Exception(), result_type); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/code_evaluator.h b/src/v8_py_frontend/code_evaluator.h index 570ff466..9053af30 100644 --- a/src/v8_py_frontend/code_evaluator.h +++ b/src/v8_py_frontend/code_evaluator.h @@ -1,11 +1,7 @@ #ifndef INCLUDE_MINI_RACER_CODE_EVALUATOR_H #define INCLUDE_MINI_RACER_CODE_EVALUATOR_H -#include -#include -#include -#include -#include "context_holder.h" +#include "isolate_manager.h" #include "isolate_memory_monitor.h" #include "value.h" @@ -14,14 +10,14 @@ namespace MiniRacer { /** Parse and run arbitrary scripts within an isolate. */ class CodeEvaluator { public: - CodeEvaluator(ContextHolder* context, + CodeEvaluator(IsolateManager* isolate_manager, ValueFactory* val_factory, IsolateMemoryMonitor* memory_monitor); - auto Eval(v8::Isolate* isolate, Value* code_ptr) -> Value::Ptr; + auto Eval(Value* code_ptr) -> Value::Ptr; private: - ContextHolder* context_; + IsolateManager* isolate_manager_; ValueFactory* val_factory_; IsolateMemoryMonitor* memory_monitor_; }; diff --git a/src/v8_py_frontend/context.cc b/src/v8_py_frontend/context.cc index 67654408..24a6d5f8 100644 --- a/src/v8_py_frontend/context.cc +++ b/src/v8_py_frontend/context.cc @@ -1,17 +1,13 @@ #include "context.h" #include -#include -#include -#include #include #include #include -#include +#include #include #include "callback.h" #include "cancelable_task_runner.h" #include "code_evaluator.h" -#include "context_holder.h" #include "heap_reporter.h" #include "isolate_manager.h" #include "isolate_memory_monitor.h" @@ -22,20 +18,19 @@ namespace MiniRacer { Context::Context(v8::Platform* platform, RawCallback callback) - : isolate_manager_(platform), - isolate_object_collector_(&isolate_manager_), + : callback_(callback), + isolate_manager_(platform), isolate_memory_monitor_(&isolate_manager_), - val_factory_(isolate_manager_.Get(), &isolate_object_collector_), - callback_([this, callback](uint64_t callback_id, Value::Ptr val) { - callback(callback_id, val_registry_.Remember(std::move(val))); - }), - context_holder_(&isolate_manager_), - js_callback_maker_(&context_holder_, &val_factory_, callback_), - code_evaluator_(&context_holder_, + val_factory_(&isolate_manager_), + js_callback_maker_(&isolate_manager_, + &val_factory_, + &val_registry_, + callback_), + code_evaluator_(&isolate_manager_, &val_factory_, &isolate_memory_monitor_), - heap_reporter_(&val_factory_), - object_manipulator_(&context_holder_, &val_factory_), + heap_reporter_(&isolate_manager_, &val_factory_), + object_manipulator_(&isolate_manager_, &val_factory_), cancelable_task_manager_(&isolate_manager_) {} Context::~Context() { @@ -45,79 +40,51 @@ Context::~Context() { } auto Context::MakeJSCallback(uint64_t callback_id) -> ValueHandle* { - return val_registry_.Remember( - isolate_manager_ - .Run([this, callback_id](v8::Isolate* isolate) { - return js_callback_maker_.MakeJSCallback(isolate, callback_id); - }) - .get()); + return val_registry_.Remember(isolate_manager_ + .Schedule([this, callback_id]() { + return js_callback_maker_.MakeJSCallback( + callback_id); + }) + .get()); } template auto Context::RunTask(Runnable runnable, uint64_t callback_id) -> uint64_t { // Start an async task! - return cancelable_task_manager_.Schedule( /*runnable=*/ std::move(runnable), /*on_completed=*/ - [this, callback_id](const Value::Ptr& val) { - callback_(callback_id, val); - }, + [this, callback_id](ValueHandle* val) { callback_(callback_id, val); }, /*on_canceled=*/ - [this, callback_id](const Value::Ptr& /*val*/) { - auto err = - val_factory_.New("execution terminated", type_terminated_exception); - callback_(callback_id, err); - }); -} - -auto Context::MakeHandleConverter(ValueHandle* handle, - const char* err_msg) -> ValueHandleConverter { - return {&val_factory_, &val_registry_, handle, err_msg}; -} - -ValueHandleConverter::ValueHandleConverter(ValueFactory* val_factory, - ValueRegistry* val_registry, - ValueHandle* handle, - const char* err_msg) - : val_registry_(val_registry), - ptr_(val_registry->FromHandle(handle)), - err_([&val_factory, err_msg, this]() { - if (ptr_) { - return Value::Ptr(); + [this, callback_id](ValueHandle* val) { + if (val == nullptr) { + callback_(callback_id, + val_registry_.Remember(val_factory_.NewFromString( + "execution terminated", type_terminated_exception))); + return; } - return val_factory->New(err_msg, type_value_exception); - }()) {} - -ValueHandleConverter::operator bool() const { - return static_cast(ptr_); -} - -auto ValueHandleConverter::GetErrorPtr() -> Value::Ptr { - return err_; -} - -auto ValueHandleConverter::GetErrorHandle() -> ValueHandle* { - return val_registry_->Remember(err_); + // This may happen if the task was mid-execution when we interrupted + // it. + callback_(callback_id, val); + }); } -auto ValueHandleConverter::GetPtr() -> Value::Ptr { - return ptr_; +auto Context::MakeHandleError(const char* err_msg) -> ValueHandle* { + return val_registry_.Remember( + val_factory_.NewFromString(err_msg, type_value_exception)); } auto Context::Eval(ValueHandle* code_handle, uint64_t callback_id) -> uint64_t { - auto code_hc = MakeHandleConverter(code_handle, "Bad handle: code"); - if (!code_hc) { - return RunTask( - [err = code_hc.GetErrorPtr()](v8::Isolate* /*isolate*/) { return err; }, - callback_id); - } - return RunTask( - [code_ptr = code_hc.GetPtr(), this](v8::Isolate* isolate) { - return code_evaluator_.Eval(isolate, code_ptr.get()); + [this, code_handle]() { + auto* code_value = val_registry_.FromHandle(code_handle); + if (code_value == nullptr) { + return MakeHandleError("Bad handle: code"); + } + + return val_registry_.Remember(code_evaluator_.Eval(code_value)); }, callback_id); } @@ -127,214 +94,219 @@ void Context::CancelTask(uint64_t task_id) { } auto Context::HeapSnapshot() -> ValueHandle* { - return val_registry_.Remember(isolate_manager_ - .Run([this](v8::Isolate* isolate) mutable { - return heap_reporter_.HeapSnapshot( - isolate); - }) - .get()); + return isolate_manager_ + .Schedule([this]() mutable { + return val_registry_.Remember(heap_reporter_.HeapSnapshot()); + }) + .get(); } auto Context::HeapStats() -> ValueHandle* { - return val_registry_.Remember(isolate_manager_ - .Run([this](v8::Isolate* isolate) mutable { - return heap_reporter_.HeapStats(isolate); - }) - .get()); + return isolate_manager_ + .Schedule([this]() mutable { + return val_registry_.Remember(heap_reporter_.HeapStats()); + }) + .get(); } auto Context::GetIdentityHash(ValueHandle* obj_handle) -> ValueHandle* { - auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); - if (!obj_hc) { - return obj_hc.GetErrorHandle(); - } + return isolate_manager_ + .Schedule([this, obj_handle]() { + auto* obj_value = val_registry_.FromHandle(obj_handle); + if (obj_value == nullptr) { + return MakeHandleError("Bad handle: obj"); + } - return val_registry_.Remember( - isolate_manager_ - .Run([this, obj_ptr = obj_hc.GetPtr()](v8::Isolate* isolate) { - return object_manipulator_.GetIdentityHash(isolate, obj_ptr.get()); - }) - .get()); + return val_registry_.Remember( + object_manipulator_.GetIdentityHash(obj_value)); + }) + .get(); } auto Context::GetOwnPropertyNames(ValueHandle* obj_handle) -> ValueHandle* { - auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); - if (!obj_hc) { - return obj_hc.GetErrorHandle(); - } + return isolate_manager_ + .Schedule([this, obj_handle]() { + auto* obj_value = val_registry_.FromHandle(obj_handle); + if (obj_value == nullptr) { + return MakeHandleError("Bad handle: obj"); + } - return val_registry_.Remember( - isolate_manager_ - .Run([this, obj_ptr = obj_hc.GetPtr()](v8::Isolate* isolate) { - return object_manipulator_.GetOwnPropertyNames(isolate, - obj_ptr.get()); - }) - .get()); + return val_registry_.Remember( + object_manipulator_.GetOwnPropertyNames(obj_value)); + }) + .get(); } auto Context::GetObjectItem(ValueHandle* obj_handle, ValueHandle* key_handle) -> ValueHandle* { - auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); - if (!obj_hc) { - return obj_hc.GetErrorHandle(); - } + return isolate_manager_ + .Schedule([this, obj_handle, key_handle]() mutable { + auto* obj_value = val_registry_.FromHandle(obj_handle); + if (obj_value == nullptr) { + return MakeHandleError("Bad handle: obj"); + } - auto key_hc = MakeHandleConverter(key_handle, "Bad handle: key"); - if (!key_hc) { - return key_hc.GetErrorHandle(); - } + auto* key_value = val_registry_.FromHandle(key_handle); + if (key_value == nullptr) { + return MakeHandleError("Bad handle: key"); + } - return val_registry_.Remember( - isolate_manager_ - .Run([this, obj_ptr = obj_hc.GetPtr(), - key_ptr = key_hc.GetPtr()](v8::Isolate* isolate) mutable { - return object_manipulator_.Get(isolate, obj_ptr.get(), - key_ptr.get()); - }) - .get()); + return val_registry_.Remember( + object_manipulator_.Get(obj_value, key_value)); + }) + .get(); } auto Context::SetObjectItem(ValueHandle* obj_handle, ValueHandle* key_handle, ValueHandle* val_handle) -> ValueHandle* { - auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); - if (!obj_hc) { - return obj_hc.GetErrorHandle(); - } + return isolate_manager_ + .Schedule([this, obj_handle, key_handle, val_handle]() mutable { + auto* obj_value = val_registry_.FromHandle(obj_handle); + if (obj_value == nullptr) { + return MakeHandleError("Bad handle: obj"); + } - auto key_hc = MakeHandleConverter(key_handle, "Bad handle: key"); - if (!key_hc) { - return key_hc.GetErrorHandle(); - } + auto* key_value = val_registry_.FromHandle(key_handle); + if (key_value == nullptr) { + return MakeHandleError("Bad handle: key"); + } - auto val_hc = MakeHandleConverter(val_handle, "Bad handle: val"); - if (!val_hc) { - return val_hc.GetErrorHandle(); - } + auto* val_value = val_registry_.FromHandle(val_handle); + if (val_value == nullptr) { + return MakeHandleError("Bad handle: val"); + } - return val_registry_.Remember( - isolate_manager_ - .Run([this, obj_ptr = obj_hc.GetPtr(), key_ptr = key_hc.GetPtr(), - val_ptr = val_hc.GetPtr()](v8::Isolate* isolate) mutable { - return object_manipulator_.Set(isolate, obj_ptr.get(), - key_ptr.get(), val_ptr.get()); - }) - .get()); + return val_registry_.Remember( + object_manipulator_.Set(obj_value, key_value, val_value)); + }) + .get(); } auto Context::DelObjectItem(ValueHandle* obj_handle, ValueHandle* key_handle) -> ValueHandle* { - auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); - if (!obj_hc) { - return obj_hc.GetErrorHandle(); - } + return isolate_manager_ + .Schedule([this, obj_handle, key_handle]() mutable { + auto* obj_value = val_registry_.FromHandle(obj_handle); + if (obj_value == nullptr) { + return MakeHandleError("Bad handle: obj"); + } - auto key_hc = MakeHandleConverter(key_handle, "Bad handle: key"); - if (!key_hc) { - return key_hc.GetErrorHandle(); - } + auto* key_value = val_registry_.FromHandle(key_handle); + if (key_value == nullptr) { + return MakeHandleError("Bad handle: key"); + } - return val_registry_.Remember( - isolate_manager_ - .Run([this, obj_ptr = obj_hc.GetPtr(), - key_ptr = key_hc.GetPtr()](v8::Isolate* isolate) mutable { - return object_manipulator_.Del(isolate, obj_ptr.get(), - key_ptr.get()); - }) - .get()); + return val_registry_.Remember( + object_manipulator_.Del(obj_value, key_value)); + }) + .get(); } auto Context::SpliceArray(ValueHandle* obj_handle, int32_t start, int32_t delete_count, ValueHandle* new_val_handle) -> ValueHandle* { - auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); - if (!obj_hc) { - return obj_hc.GetErrorHandle(); - } - - Value::Ptr new_val_ptr; - if (new_val_handle != nullptr) { - auto new_val_hc = - MakeHandleConverter(new_val_handle, "Bad handle: new_val"); - if (!new_val_hc) { - return new_val_hc.GetErrorHandle(); - } - new_val_ptr = new_val_hc.GetPtr(); - } + return isolate_manager_ + .Schedule([this, obj_handle, start, delete_count, new_val_handle]() { + auto* obj_value = val_registry_.FromHandle(obj_handle); + if (obj_value == nullptr) { + return MakeHandleError("Bad handle: obj"); + } - return val_registry_.Remember( - isolate_manager_ - .Run([this, obj_ptr = obj_hc.GetPtr(), start, delete_count, - new_val_ptr](v8::Isolate* isolate) { - return object_manipulator_.Splice(isolate, obj_ptr.get(), start, - delete_count, new_val_ptr.get()); - }) - .get()); + Value* new_val_value = nullptr; + if (new_val_handle != nullptr) { + new_val_value = val_registry_.FromHandle(new_val_handle); + if (new_val_value == nullptr) { + return MakeHandleError("Bad handle: new_value"); + } + } + + return val_registry_.Remember(object_manipulator_.Splice( + obj_value, start, delete_count, new_val_value)); + }) + .get(); } auto Context::ArrayPush(ValueHandle* obj_handle, ValueHandle* new_val_handle) -> ValueHandle* { - auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); - if (!obj_hc) { - return obj_hc.GetErrorHandle(); - } + return isolate_manager_ + .Schedule([this, obj_handle, new_val_handle]() { + auto* obj_value = val_registry_.FromHandle(obj_handle); + if (obj_value == nullptr) { + return MakeHandleError("Bad handle: obj"); + } - auto new_val_hc = MakeHandleConverter(new_val_handle, "Bad handle: new_val"); - if (!new_val_hc) { - return new_val_hc.GetErrorHandle(); - } + auto* new_val_value = val_registry_.FromHandle(new_val_handle); + if (new_val_value == nullptr) { + return MakeHandleError("Bad handle: new_val"); + } - return val_registry_.Remember( - isolate_manager_ - .Run([this, obj_ptr = obj_hc.GetPtr(), - new_val_ptr = new_val_hc.GetPtr()](v8::Isolate* isolate) { - return object_manipulator_.Push(isolate, obj_ptr.get(), - new_val_ptr.get()); - }) - .get()); + return val_registry_.Remember( + object_manipulator_.Push(obj_value, new_val_value)); + }) + .get(); } void Context::FreeValue(ValueHandle* val) { - val_registry_.Forget(val); + isolate_manager_.Schedule([this, val]() { val_registry_.Forget(val); }).get(); +} + +auto Context::AllocInt(int64_t val, ValueTypes type) -> ValueHandle* { + return isolate_manager_ + .Schedule([this, val, type]() { + return val_registry_.Remember(val_factory_.NewFromInt(val, type)); + }) + .get(); +} + +auto Context::AllocDouble(double val, ValueTypes type) -> ValueHandle* { + return isolate_manager_ + .Schedule([this, val, type]() { + return val_registry_.Remember(val_factory_.NewFromDouble(val, type)); + }) + .get(); +} + +auto Context::AllocString(std::string_view val, + ValueTypes type) -> ValueHandle* { + return isolate_manager_ + .Schedule([this, val, type]() { + return val_registry_.Remember(val_factory_.NewFromString(val, type)); + }) + .get(); } auto Context::CallFunction(ValueHandle* func_handle, ValueHandle* this_handle, ValueHandle* argv_handle, uint64_t callback_id) -> uint64_t { - auto func_hc = MakeHandleConverter(func_handle, "Bad handle: func"); - if (!func_hc) { - return RunTask( - [err = func_hc.GetErrorPtr()](v8::Isolate* /*isolate*/) { return err; }, - callback_id); - } - - auto this_hc = MakeHandleConverter(this_handle, "Bad handle: this"); - if (!this_hc) { - return RunTask( - [err = this_hc.GetErrorPtr()](v8::Isolate* /*isolate*/) { return err; }, - callback_id); - } - - auto argv_hc = MakeHandleConverter(argv_handle, "Bad handle: argv"); - if (!argv_hc) { - return RunTask( - [err = argv_hc.GetErrorPtr()](v8::Isolate* /*isolate*/) { return err; }, - callback_id); - } - return RunTask( - [this, func_ptr = func_hc.GetPtr(), this_ptr = this_hc.GetPtr(), - argv_ptr = argv_hc.GetPtr()](v8::Isolate* isolate) { - return object_manipulator_.Call(isolate, func_ptr.get(), this_ptr.get(), - argv_ptr.get()); + [this, func_handle, this_handle, argv_handle]() { + auto* func_value = val_registry_.FromHandle(func_handle); + if (func_value == nullptr) { + return MakeHandleError("Bad handle: func"); + } + + auto* this_value = val_registry_.FromHandle(this_handle); + if (this_value == nullptr) { + return MakeHandleError("Bad handle: this"); + } + + auto* argv_value = val_registry_.FromHandle(argv_handle); + if (argv_value == nullptr) { + return MakeHandleError("Bad handle: argv"); + } + + return val_registry_.Remember( + object_manipulator_.Call(func_value, this_value, argv_value)); }, callback_id); } auto Context::ValueCount() -> size_t { - return val_registry_.Count(); + return isolate_manager_.Schedule([this]() { return val_registry_.Count(); }) + .get(); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/context.h b/src/v8_py_frontend/context.h index deb9fd4a..6b60a805 100644 --- a/src/v8_py_frontend/context.h +++ b/src/v8_py_frontend/context.h @@ -4,14 +4,13 @@ #include #include #include +#include #include "callback.h" #include "cancelable_task_runner.h" #include "code_evaluator.h" -#include "context_holder.h" #include "heap_reporter.h" #include "isolate_manager.h" #include "isolate_memory_monitor.h" -#include "isolate_object_collector.h" #include "js_callback_maker.h" #include "object_manipulator.h" #include "value.h" @@ -38,8 +37,9 @@ class Context { void ApplyLowMemoryNotification(); void FreeValue(ValueHandle* val); - template - auto AllocValue(Params&&... params) -> ValueHandle*; + auto AllocInt(int64_t val, ValueTypes type) -> ValueHandle*; + auto AllocDouble(double val, ValueTypes type) -> ValueHandle*; + auto AllocString(std::string_view val, ValueTypes type) -> ValueHandle*; void CancelTask(uint64_t task_id); auto HeapSnapshot() -> ValueHandle*; auto HeapStats() -> ValueHandle*; @@ -72,16 +72,13 @@ class Context { template auto RunTask(Runnable runnable, uint64_t callback_id) -> uint64_t; - auto MakeHandleConverter(ValueHandle* handle, - const char* err_msg) -> ValueHandleConverter; + auto MakeHandleError(const char* err_msg) -> ValueHandle*; + RawCallback callback_; IsolateManager isolate_manager_; - IsolateObjectCollector isolate_object_collector_; IsolateMemoryMonitor isolate_memory_monitor_; ValueFactory val_factory_; ValueRegistry val_registry_; - CallbackFn callback_; - ContextHolder context_holder_; JSCallbackMaker js_callback_maker_; CodeEvaluator code_evaluator_; HeapReporter heap_reporter_; @@ -89,25 +86,6 @@ class Context { CancelableTaskManager cancelable_task_manager_; }; -class ValueHandleConverter { - public: - ValueHandleConverter(ValueFactory* val_factory, - ValueRegistry* val_registry, - ValueHandle* handle, - const char* err_msg); - - explicit operator bool() const; - - auto GetErrorPtr() -> Value::Ptr; - auto GetErrorHandle() -> ValueHandle*; - auto GetPtr() -> Value::Ptr; - - private: - ValueRegistry* val_registry_; - Value::Ptr ptr_; - Value::Ptr err_; -}; - inline void Context::SetHardMemoryLimit(size_t limit) { isolate_memory_monitor_.SetHardMemoryLimit(limit); } @@ -128,12 +106,6 @@ inline void Context::ApplyLowMemoryNotification() { isolate_memory_monitor_.ApplyLowMemoryNotification(); } -template -inline auto Context::AllocValue(Params&&... params) -> ValueHandle* { - return val_registry_.Remember( - val_factory_.New(std::forward(params)...)); -} - } // namespace MiniRacer #endif // INCLUDE_MINI_RACER_CONTEXT_H diff --git a/src/v8_py_frontend/context_holder.cc b/src/v8_py_frontend/context_holder.cc deleted file mode 100644 index e74f137f..00000000 --- a/src/v8_py_frontend/context_holder.cc +++ /dev/null @@ -1,31 +0,0 @@ -#include "context_holder.h" - -#include -#include -#include -#include -#include -#include -#include "isolate_manager.h" - -namespace MiniRacer { - -ContextHolder::ContextHolder(IsolateManager* isolate_manager) - : isolate_manager_(isolate_manager), - context_(isolate_manager_ - ->Run([](v8::Isolate* isolate) { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - - return std::make_unique>( - isolate, v8::Context::New(isolate)); - }) - .get()) {} - -ContextHolder::~ContextHolder() { - isolate_manager_ - ->Run([context = std::move(context_)](v8::Isolate*) { context->Reset(); }) - .get(); -} - -} // end namespace MiniRacer diff --git a/src/v8_py_frontend/context_holder.h b/src/v8_py_frontend/context_holder.h deleted file mode 100644 index 35f3754a..00000000 --- a/src/v8_py_frontend/context_holder.h +++ /dev/null @@ -1,35 +0,0 @@ -#ifndef INCLUDE_MINI_RACER_CONTEXT_HOLDER_H -#define INCLUDE_MINI_RACER_CONTEXT_HOLDER_H - -#include -#include -#include -#include "isolate_manager.h" - -namespace MiniRacer { - -/** Create and manage a v8::Context */ -class ContextHolder { - public: - explicit ContextHolder(IsolateManager* isolate_manager); - ~ContextHolder(); - - ContextHolder(const ContextHolder&) = delete; - auto operator=(const ContextHolder&) -> ContextHolder& = delete; - ContextHolder(ContextHolder&&) = delete; - auto operator=(ContextHolder&& other) -> ContextHolder& = delete; - - auto Get() -> v8::Global*; - - private: - IsolateManager* isolate_manager_; - std::unique_ptr> context_; -}; - -inline auto ContextHolder::Get() -> v8::Global* { - return context_.get(); -} - -} // end namespace MiniRacer - -#endif // INCLUDE_MINI_RACER_CONTEXT_HOLDER_H diff --git a/src/v8_py_frontend/exports.cc b/src/v8_py_frontend/exports.cc index 21b79bcf..425f10d0 100644 --- a/src/v8_py_frontend/exports.cc +++ b/src/v8_py_frontend/exports.cc @@ -81,7 +81,7 @@ LIB_EXPORT auto mr_alloc_int_val(uint64_t context_id, if (!context) { return nullptr; } - return context->AllocValue(val, type); + return context->AllocInt(val, type); } LIB_EXPORT auto mr_alloc_double_val(uint64_t context_id, @@ -92,7 +92,7 @@ LIB_EXPORT auto mr_alloc_double_val(uint64_t context_id, if (!context) { return nullptr; } - return context->AllocValue(val, type); + return context->AllocDouble(val, type); } LIB_EXPORT auto mr_alloc_string_val(uint64_t context_id, @@ -104,7 +104,7 @@ LIB_EXPORT auto mr_alloc_string_val(uint64_t context_id, if (!context) { return nullptr; } - return context->AllocValue(std::string_view(val, len), type); + return context->AllocString(std::string_view(val, len), type); } LIB_EXPORT void mr_cancel_task(uint64_t context_id, uint64_t task_id) { diff --git a/src/v8_py_frontend/exports.h b/src/v8_py_frontend/exports.h index 78ff8bd4..520f33ff 100644 --- a/src/v8_py_frontend/exports.h +++ b/src/v8_py_frontend/exports.h @@ -110,7 +110,7 @@ LIB_EXPORT auto mr_alloc_double_val(uint64_t context_id, /** Allocate a ValueHandle pointing to a copy of the given utf-8 string. * * If used as an argument in another call, this value will be rendered into - * JavaScript as an ordinary string. Only type type_str_utf8 is supported. + * JavaScript as an ordinary string. **/ LIB_EXPORT auto mr_alloc_string_val(uint64_t context_id, char* val, diff --git a/src/v8_py_frontend/heap_reporter.cc b/src/v8_py_frontend/heap_reporter.cc index f7d02fbb..5340de74 100644 --- a/src/v8_py_frontend/heap_reporter.cc +++ b/src/v8_py_frontend/heap_reporter.cc @@ -1,5 +1,6 @@ #include "heap_reporter.h" #include +#include #include #include #include @@ -7,18 +8,20 @@ #include #include #include +#include "isolate_manager.h" #include "value.h" namespace MiniRacer { -HeapReporter::HeapReporter(ValueFactory* val_factory) - : val_factory_(val_factory) {} +HeapReporter::HeapReporter(IsolateManager* isolate_manager, + ValueFactory* val_factory) + : isolate_manager_(isolate_manager), val_factory_(val_factory) {} -auto HeapReporter::HeapStats(v8::Isolate* isolate) -> Value::Ptr { - const v8::Isolate::Scope isolatescope(isolate); - const v8::HandleScope handle_scope(isolate); +auto HeapReporter::HeapStats() -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); const v8::TryCatch trycatch(isolate); + const v8::Local context = v8::Context::New(isolate); const v8::Context::Scope context_scope(context); @@ -59,9 +62,10 @@ auto HeapReporter::HeapStats(v8::Isolate* isolate) -> Value::Ptr { v8::Local output; if (!v8::JSON::Stringify(context, stats_obj).ToLocal(&output) || output.IsEmpty()) { - return val_factory_->New("error stringifying heap output", type_str_utf8); + return val_factory_->NewFromString("error stringifying heap output", + type_string); } - return val_factory_->New(context, output); + return val_factory_->NewFromAny(output); } namespace { @@ -82,13 +86,12 @@ class StringOutputStream : public v8::OutputStream { }; } // end anonymous namespace -auto HeapReporter::HeapSnapshot(v8::Isolate* isolate) -> Value::Ptr { - const v8::Isolate::Scope isolatescope(isolate); - const v8::HandleScope handle_scope(isolate); - const auto* snap = isolate->GetHeapProfiler()->TakeHeapSnapshot(); +auto HeapReporter::HeapSnapshot() -> Value::Ptr { + const auto* snap = + isolate_manager_->GetIsolate()->GetHeapProfiler()->TakeHeapSnapshot(); StringOutputStream sos; snap->Serialize(&sos); - return val_factory_->New(sos.result(), type_str_utf8); + return val_factory_->NewFromString(sos.result(), type_string); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/heap_reporter.h b/src/v8_py_frontend/heap_reporter.h index 46285a35..c6551b24 100644 --- a/src/v8_py_frontend/heap_reporter.h +++ b/src/v8_py_frontend/heap_reporter.h @@ -1,7 +1,7 @@ #ifndef INCLUDE_MINI_RACER_HEAP_REPORTER_H #define INCLUDE_MINI_RACER_HEAP_REPORTER_H -#include +#include "isolate_manager.h" #include "value.h" namespace MiniRacer { @@ -9,12 +9,13 @@ namespace MiniRacer { /** Report fun facts about an isolate heap */ class HeapReporter { public: - explicit HeapReporter(ValueFactory* val_factory); + HeapReporter(IsolateManager* isolate_manager, ValueFactory* val_factory); - auto HeapSnapshot(v8::Isolate* isolate) -> Value::Ptr; - auto HeapStats(v8::Isolate* isolate) -> Value::Ptr; + auto HeapSnapshot() -> Value::Ptr; + auto HeapStats() -> Value::Ptr; private: + IsolateManager* isolate_manager_; ValueFactory* val_factory_; }; diff --git a/src/v8_py_frontend/isolate_holder.cc b/src/v8_py_frontend/isolate_holder.cc deleted file mode 100644 index 1aab56b4..00000000 --- a/src/v8_py_frontend/isolate_holder.cc +++ /dev/null @@ -1,26 +0,0 @@ -#include "isolate_holder.h" - -#include -#include -#include - -namespace MiniRacer { - -IsolateHolder::IsolateHolder() - : allocator_(v8::ArrayBuffer::Allocator::NewDefaultAllocator()) { - v8::Isolate::CreateParams create_params; - create_params.array_buffer_allocator = allocator_.get(); - - isolate_ = v8::Isolate::New(create_params); - - // We should set kExplicit since we're running the Microtasks checkpoint - // manually in isolate_manager.cc. Per - // https://stackoverflow.com/questions/54393127/v8-how-to-correctly-handle-microtasks - isolate_->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit); -} - -IsolateHolder::~IsolateHolder() { - isolate_->Dispose(); -} - -} // end namespace MiniRacer diff --git a/src/v8_py_frontend/isolate_holder.h b/src/v8_py_frontend/isolate_holder.h deleted file mode 100644 index 6148bca1..00000000 --- a/src/v8_py_frontend/isolate_holder.h +++ /dev/null @@ -1,34 +0,0 @@ -#ifndef INCLUDE_MINI_RACER_ISOLATE_HOLDER_H -#define INCLUDE_MINI_RACER_ISOLATE_HOLDER_H - -#include -#include -#include - -namespace MiniRacer { - -/** Create and manage lifecycle of a v8::Isolate */ -class IsolateHolder { - public: - IsolateHolder(); - ~IsolateHolder(); - - IsolateHolder(const IsolateHolder&) = delete; - auto operator=(const IsolateHolder&) -> IsolateHolder& = delete; - IsolateHolder(IsolateHolder&&) = delete; - auto operator=(IsolateHolder&& other) -> IsolateHolder& = delete; - - auto Get() -> v8::Isolate*; - - private: - std::unique_ptr allocator_; - v8::Isolate* isolate_; -}; - -inline auto IsolateHolder::Get() -> v8::Isolate* { - return isolate_; -} - -} // end namespace MiniRacer - -#endif // INCLUDE_MINI_RACER_ISOLATE_HOLDER_H diff --git a/src/v8_py_frontend/isolate_manager.cc b/src/v8_py_frontend/isolate_manager.cc index f985ffe7..a165e8e3 100644 --- a/src/v8_py_frontend/isolate_manager.cc +++ b/src/v8_py_frontend/isolate_manager.cc @@ -3,25 +3,39 @@ #include #include #include +#include #include #include #include -#include "isolate_holder.h" namespace MiniRacer { IsolateManager::IsolateManager(v8::Platform* platform) : platform_(platform), state_(State::kRun), + allocator_(v8::ArrayBuffer::Allocator::NewDefaultAllocator()), + isolate_([this]() -> v8::Isolate* { + v8::Isolate::CreateParams create_params; + create_params.array_buffer_allocator = allocator_.get(); + + v8::Isolate* isolate = v8::Isolate::New(create_params); + + // We should set kExplicit since we're running the Microtasks checkpoint + // manually in isolate_manager.cc. Per + // https://stackoverflow.com/questions/54393127/v8-how-to-correctly-handle-microtasks + isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit); + return isolate; + }()), thread_([this]() { PumpMessages(); }) {} IsolateManager::~IsolateManager() { ChangeState(State::kStop); thread_.join(); + isolate_->Dispose(); } void IsolateManager::TerminateOngoingTask() { - isolate_holder_.Get()->TerminateExecution(); + isolate_->TerminateExecution(); } void IsolateManager::StopJavaScript() { @@ -32,34 +46,38 @@ void IsolateManager::StopJavaScript() { void IsolateManager::PumpMessages() { // By design, only this, the message pump thread, is ever allowed to touch // the isolate, so go ahead and lock it: - v8::Isolate* isolate = isolate_holder_.Get(); - const v8::Locker lock(isolate); - const v8::Isolate::Scope scope(isolate); + const v8::Locker lock(isolate_); + const v8::Isolate::Scope scope(isolate_); + const v8::HandleScope handle_scope(isolate_); - const v8::SealHandleScope shs(isolate); + context_.Reset(isolate_, v8::Context::New(isolate_)); + + const v8::SealHandleScope shs(isolate_); while (state_ == State::kRun) { v8::platform::PumpMessageLoop( - platform_, isolate, v8::platform::MessageLoopBehavior::kWaitForWork); + platform_, isolate_, v8::platform::MessageLoopBehavior::kWaitForWork); if (state_ == State::kRun) { - isolate->PerformMicrotaskCheckpoint(); + isolate_->PerformMicrotaskCheckpoint(); } } const v8::Isolate::DisallowJavascriptExecutionScope disallow_js( - isolate, v8::Isolate::DisallowJavascriptExecutionScope::OnFailure:: - THROW_ON_FAILURE); + isolate_, v8::Isolate::DisallowJavascriptExecutionScope::OnFailure:: + THROW_ON_FAILURE); while (state_ == State::kNoJavaScript) { v8::platform::PumpMessageLoop( - platform_, isolate, v8::platform::MessageLoopBehavior::kWaitForWork); + platform_, isolate_, v8::platform::MessageLoopBehavior::kWaitForWork); } + + context_.Reset(); } void IsolateManager::ChangeState(State state) { state_ = state; // Run a no-op task to kick the message loop into noticing we've switched // states: - std::ignore = Run([](v8::Isolate*) {}); + std::ignore = Schedule([]() {}); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/isolate_manager.h b/src/v8_py_frontend/isolate_manager.h index b76aad5f..2177dcd9 100644 --- a/src/v8_py_frontend/isolate_manager.h +++ b/src/v8_py_frontend/isolate_manager.h @@ -1,7 +1,11 @@ #ifndef INCLUDE_MINI_RACER_ISOLATE_MANAGER_H #define INCLUDE_MINI_RACER_ISOLATE_MANAGER_H +#include +#include #include +#include +#include #include #include #include @@ -10,27 +14,28 @@ #include #include #include -#include "isolate_holder.h" namespace MiniRacer { +class IsolateManager; + /** Wraps up a runnable to run on a v8::Isolate's foreground task runner thread * . */ template class IsolateTask : public v8::Task { public: - using ReturnType = std::invoke_result_t; + using ReturnType = std::invoke_result_t; using FutureType = std::future; - explicit IsolateTask(Runnable runnable, v8::Isolate* isolate); + explicit IsolateTask(Runnable runnable, IsolateManager* isolate_manager); void Run() override; auto GetFuture() -> FutureType; private: - std::packaged_task packaged_task_; - v8::Isolate* isolate_; + std::packaged_task packaged_task_; + IsolateManager* isolate_manager_; }; /** Owns a v8::Isolate and mediates access to it via a task queue. @@ -55,7 +60,8 @@ class IsolateManager { IsolateManager(IsolateManager&&) = delete; auto operator=(IsolateManager&& other) -> IsolateManager& = delete; - auto Get() -> v8::Isolate*; + auto GetIsolate() -> v8::Isolate*; + auto GetLocalContext() -> v8::Local; /** Schedules a task to run on the foreground thread, using * v8::TaskRunner::PostTask. Returns a future which gets the result. @@ -63,8 +69,8 @@ class IsolateManager { * runnable outlive the task, by awaiting the returned future before tearing * down any referred-to objects. */ template - [[nodiscard]] auto Run(Runnable runnable) - -> IsolateTask::FutureType; + [[nodiscard]] auto Schedule(Runnable runnable) -> + typename IsolateTask::FutureType; void TerminateOngoingTask(); @@ -82,38 +88,48 @@ class IsolateManager { v8::Platform* platform_; std::atomic state_; - IsolateHolder isolate_holder_; + std::unique_ptr allocator_; + v8::Isolate* isolate_; std::thread thread_; + v8::Global context_; }; -inline auto IsolateManager::Get() -> v8::Isolate* { - return isolate_holder_.Get(); +inline auto IsolateManager::GetIsolate() -> v8::Isolate* { + return isolate_; +} + +inline auto IsolateManager::GetLocalContext() -> v8::Local { + return context_.Get(isolate_); } /** Schedules a task to run on the foreground thread, using * v8::TaskRunner::PostTask. Awaits task completion. */ template -inline auto IsolateManager::Run(Runnable runnable) - -> IsolateTask::FutureType { - auto task = std::make_unique>(std::move(runnable), - isolate_holder_.Get()); +inline auto IsolateManager::Schedule(Runnable runnable) -> + typename IsolateTask::FutureType { + auto task = + std::make_unique>(std::move(runnable), this); auto fut = task->GetFuture(); - platform_->GetForegroundTaskRunner(isolate_holder_.Get()) - ->PostTask(std::move(task)); + platform_->GetForegroundTaskRunner(isolate_)->PostTask(std::move(task)); return fut; } template inline IsolateTask::IsolateTask(Runnable runnable, - v8::Isolate* isolate) - : packaged_task_(std::move(runnable)), isolate_(isolate) {} + IsolateManager* isolate_manager) + : packaged_task_(std::move(runnable)), isolate_manager_(isolate_manager) {} template inline void IsolateTask::Run() { - packaged_task_(isolate_); + v8::Isolate* isolate = isolate_manager_->GetIsolate(); + const v8::HandleScope handle_scope(isolate); + const v8::Local context = isolate_manager_->GetLocalContext(); + const v8::Context::Scope context_scope(context); + + packaged_task_(); } template diff --git a/src/v8_py_frontend/isolate_memory_monitor.cc b/src/v8_py_frontend/isolate_memory_monitor.cc index 2a7ff6d7..11f625ff 100644 --- a/src/v8_py_frontend/isolate_memory_monitor.cc +++ b/src/v8_py_frontend/isolate_memory_monitor.cc @@ -12,9 +12,9 @@ IsolateMemoryMonitor::IsolateMemoryMonitor(IsolateManager* isolate_manager) : isolate_manager_(isolate_manager), state_(std::make_shared()) { isolate_manager_ - ->Run([state = state_](v8::Isolate* isolate) { - isolate->AddGCEpilogueCallback(&IsolateMemoryMonitor::StaticGCCallback, - state.get()); + ->Schedule([this]() { + isolate_manager_->GetIsolate()->AddGCEpilogueCallback( + &IsolateMemoryMonitor::StaticGCCallback, state_.get()); }) .get(); } @@ -36,15 +36,16 @@ auto IsolateMemoryMonitor::IsHardMemoryLimitReached() const -> bool { void IsolateMemoryMonitor::ApplyLowMemoryNotification() { isolate_manager_ - ->Run([](v8::Isolate* isolate) { isolate->LowMemoryNotification(); }) + ->Schedule( + [this]() { isolate_manager_->GetIsolate()->LowMemoryNotification(); }) .get(); } IsolateMemoryMonitor::~IsolateMemoryMonitor() { isolate_manager_ - ->Run([state = state_](v8::Isolate* isolate) { - isolate->RemoveGCEpilogueCallback( - &IsolateMemoryMonitor::StaticGCCallback, state.get()); + ->Schedule([this]() { + isolate_manager_->GetIsolate()->RemoveGCEpilogueCallback( + &IsolateMemoryMonitor::StaticGCCallback, state_.get()); }) .get(); } diff --git a/src/v8_py_frontend/isolate_object_collector.cc b/src/v8_py_frontend/isolate_object_collector.cc deleted file mode 100644 index 08f88291..00000000 --- a/src/v8_py_frontend/isolate_object_collector.cc +++ /dev/null @@ -1,58 +0,0 @@ -#include "isolate_object_collector.h" -#include -#include -#include -#include -#include -#include -#include -#include "isolate_manager.h" - -namespace MiniRacer { - -IsolateObjectCollector::IsolateObjectCollector(IsolateManager* isolate_manager) - : isolate_manager_(isolate_manager), is_collecting_(false) {} - -IsolateObjectCollector::~IsolateObjectCollector() { - std::unique_lock lock(mutex_); - collection_done_cv_.wait(lock, [this] { return !is_collecting_; }); -} - -void IsolateObjectCollector::EnqueueCollectionBatchLocked() { - is_collecting_ = true; - - std::ignore = isolate_manager_->Run([this](v8::Isolate*) { DoCollection(); }); -} - -void IsolateObjectCollector::DoCollection() { - std::vector> batch; - { - const std::lock_guard lock(mutex_); - batch = std::exchange(garbage_, {}); - } - - batch.clear(); - - const std::lock_guard lock(mutex_); - if (garbage_.empty()) { - is_collecting_ = false; - collection_done_cv_.notify_all(); - return; - } - - EnqueueCollectionBatchLocked(); -} - -void IsolateObjectCollector::Collect(v8::Global global) { - const std::lock_guard lock(mutex_); - - garbage_.push_back(std::move(global)); - - if (is_collecting_) { - // There is already a collection in progress. - return; - } - - EnqueueCollectionBatchLocked(); -} -} // end namespace MiniRacer diff --git a/src/v8_py_frontend/isolate_object_collector.h b/src/v8_py_frontend/isolate_object_collector.h deleted file mode 100644 index a93a9caf..00000000 --- a/src/v8_py_frontend/isolate_object_collector.h +++ /dev/null @@ -1,51 +0,0 @@ -#ifndef INCLUDE_MINI_RACER_ISOLATE_OBJECT_COLLECTOR_H -#define INCLUDE_MINI_RACER_ISOLATE_OBJECT_COLLECTOR_H - -#include -#include -#include -#include -#include -#include -#include "isolate_manager.h" - -namespace MiniRacer { - -/** Deletes v8 objects. - * - * Things that want to delete v8 objects often don't own the isolate lock - * (i.e., aren't running from the IsolateManager's message loop). From the V8 - * documentation, it's not clear if we can safely free v8 objects like a - * v8::Global handle without the lock. As a rule, messing with v8::Isolate-owned - * objects without holding the Isolate lock is not safe, and there is no - * documentation indicating methods like v8::Global::~Global are exempt from - * this rule. So this class delegates deletion to the Isolate message loop. - */ -class IsolateObjectCollector { - public: - explicit IsolateObjectCollector(IsolateManager* isolate_manager); - ~IsolateObjectCollector(); - - IsolateObjectCollector(const IsolateObjectCollector&) = delete; - auto operator=(const IsolateObjectCollector&) -> IsolateObjectCollector& = - delete; - IsolateObjectCollector(IsolateObjectCollector&&) = delete; - auto operator=(IsolateObjectCollector&& other) -> IsolateObjectCollector& = - delete; - - void Collect(v8::Global global); - - private: - void EnqueueCollectionBatchLocked(); - void DoCollection(); - - IsolateManager* isolate_manager_; - std::mutex mutex_; - std::vector> garbage_; - std::condition_variable collection_done_cv_; - bool is_collecting_; -}; - -} // namespace MiniRacer - -#endif // INCLUDE_MINI_RACER_ISOLATE_OBJECT_COLLECTOR_H diff --git a/src/v8_py_frontend/js_callback_maker.cc b/src/v8_py_frontend/js_callback_maker.cc index 4345c88c..99072bd3 100644 --- a/src/v8_py_frontend/js_callback_maker.cc +++ b/src/v8_py_frontend/js_callback_maker.cc @@ -10,23 +10,24 @@ #include #include #include -#include #include "callback.h" -#include "context_holder.h" #include "id_maker.h" +#include "isolate_manager.h" #include "value.h" namespace MiniRacer { JSCallbackCaller::JSCallbackCaller(ValueFactory* val_factory, - CallbackFn callback) - : val_factory_(val_factory), callback_(std::move(callback)) {} + ValueRegistry* val_registry, + RawCallback callback) + : val_factory_(val_factory), + val_registry_(val_registry), + callback_(callback) {} -void JSCallbackCaller::DoCallback(v8::Local context, - uint64_t callback_id, +void JSCallbackCaller::DoCallback(uint64_t callback_id, v8::Local args) { - const Value::Ptr args_ptr = val_factory_->New(context, args); - callback_(callback_id, args_ptr); + callback_(callback_id, + val_registry_->Remember(val_factory_->NewFromAny(args))); } std::shared_ptr> JSCallbackMaker::callback_callers_; @@ -40,21 +41,19 @@ auto JSCallbackMaker::GetCallbackCallers() return callback_callers_; } -JSCallbackMaker::JSCallbackMaker(ContextHolder* context_holder, +JSCallbackMaker::JSCallbackMaker(IsolateManager* isolate_manager, ValueFactory* val_factory, - CallbackFn callback) - : context_holder_(context_holder), + ValueRegistry* val_registry, + RawCallback callback) + : isolate_manager_(isolate_manager), val_factory_(val_factory), - callback_caller_holder_( - std::make_shared(val_factory, std::move(callback)), - GetCallbackCallers()) {} + callback_caller_holder_(std::make_shared(val_factory, + val_registry, + callback), + GetCallbackCallers()) {} -auto JSCallbackMaker::MakeJSCallback(v8::Isolate* isolate, - uint64_t callback_id) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local context = context_holder_->Get()->Get(isolate); - const v8::Context::Scope context_scope(context); +auto JSCallbackMaker::MakeJSCallback(uint64_t callback_id) -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); // We create a JS Array containing: // {a BigInt indicating the callback caller ID, a BigInt indicating the @@ -76,11 +75,16 @@ auto JSCallbackMaker::MakeJSCallback(v8::Isolate* isolate, const v8::Local data = v8::Array::New(isolate, data_elements.data(), data_elements.size()); - const v8::Local func = - v8::Function::New(context, &JSCallbackMaker::OnCalledStatic, data) - .ToLocalChecked(); + const v8::Local context = isolate_manager_->GetLocalContext(); - return val_factory_->New(context, func); + v8::Local func; + if (!v8::Function::New(context, &JSCallbackMaker::OnCalledStatic, data) + .ToLocal(&func)) { + return val_factory_->NewFromString("Could not create func", + type_execute_exception); + } + + return val_factory_->NewFromAny(func); } void JSCallbackMaker::OnCalledStatic( @@ -88,7 +92,6 @@ void JSCallbackMaker::OnCalledStatic( v8::Isolate* isolate = info.GetIsolate(); const v8::HandleScope scope(isolate); const v8::Local context = isolate->GetCurrentContext(); - const v8::Context::Scope context_scope(context); const v8::Local data_value = info.Data(); @@ -151,7 +154,7 @@ void JSCallbackMaker::OnCalledStatic( return; } - callback_caller->DoCallback(context, callback_id, args); + callback_caller->DoCallback(callback_id, args); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/js_callback_maker.h b/src/v8_py_frontend/js_callback_maker.h index b880a1c7..933b2001 100644 --- a/src/v8_py_frontend/js_callback_maker.h +++ b/src/v8_py_frontend/js_callback_maker.h @@ -4,14 +4,13 @@ #include #include #include -#include #include #include #include #include #include "callback.h" -#include "context_holder.h" #include "id_maker.h" +#include "isolate_manager.h" #include "value.h" namespace MiniRacer { @@ -23,26 +22,28 @@ namespace MiniRacer { */ class JSCallbackCaller { public: - JSCallbackCaller(ValueFactory* val_factory, CallbackFn callback); + JSCallbackCaller(ValueFactory* val_factory, + ValueRegistry* val_registry, + RawCallback callback); - void DoCallback(v8::Local context, - uint64_t callback_id, - v8::Local args); + void DoCallback(uint64_t callback_id, v8::Local args); private: ValueFactory* val_factory_; - CallbackFn callback_; + ValueRegistry* val_registry_; + RawCallback callback_; }; /** Creates a JS callback wrapped around the given C callback function pointer. */ class JSCallbackMaker { public: - JSCallbackMaker(ContextHolder* context_holder, + JSCallbackMaker(IsolateManager* isolate_manager, ValueFactory* val_factory, - CallbackFn callback); + ValueRegistry* val_registry, + RawCallback callback); - auto MakeJSCallback(v8::Isolate* isolate, uint64_t callback_id) -> Value::Ptr; + auto MakeJSCallback(uint64_t callback_id) -> Value::Ptr; private: static void OnCalledStatic(const v8::FunctionCallbackInfo& info); @@ -52,7 +53,7 @@ class JSCallbackMaker { static std::shared_ptr> callback_callers_; static std::once_flag callback_callers_init_flag_; - ContextHolder* context_holder_; + IsolateManager* isolate_manager_; ValueFactory* val_factory_; IdHolder callback_caller_holder_; }; diff --git a/src/v8_py_frontend/object_manipulator.cc b/src/v8_py_frontend/object_manipulator.cc index 5cfcb62e..800bc291 100644 --- a/src/v8_py_frontend/object_manipulator.cc +++ b/src/v8_py_frontend/object_manipulator.cc @@ -3,135 +3,103 @@ #include #include #include -#include #include #include -#include #include #include #include -#include "context_holder.h" +#include "isolate_manager.h" #include "value.h" namespace MiniRacer { - -ObjectManipulator::ObjectManipulator(ContextHolder* context, +ObjectManipulator::ObjectManipulator(IsolateManager* isolate_manager, ValueFactory* val_factory) - : context_(context), val_factory_(val_factory) {} + : isolate_manager_(isolate_manager), val_factory_(val_factory) {} -auto ObjectManipulator::GetIdentityHash(v8::Isolate* isolate, - Value* obj_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); +auto ObjectManipulator::GetIdentityHash(Value* obj_ptr) -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_obj_val = - obj_ptr->ToV8Value(isolate, local_context); + const v8::Local local_obj_val = obj_ptr->Global()->Get(isolate); const v8::Local local_obj = local_obj_val.As(); - return val_factory_->New(static_cast(local_obj->GetIdentityHash()), - type_integer); + return val_factory_->NewFromInt(local_obj->GetIdentityHash(), type_integer); } -auto ObjectManipulator::GetOwnPropertyNames(v8::Isolate* isolate, - Value* obj_ptr) const +auto ObjectManipulator::GetOwnPropertyNames(Value* obj_ptr) const -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_obj_val = - obj_ptr->ToV8Value(isolate, local_context); + const v8::Local local_obj_val = obj_ptr->Global()->Get(isolate); const v8::Local local_obj = local_obj_val.As(); + const v8::Local context = isolate_manager_->GetLocalContext(); + const v8::Local names = - local_obj->GetPropertyNames(local_context).ToLocalChecked(); + local_obj->GetPropertyNames(context).ToLocalChecked(); - return val_factory_->New(local_context, names); + return val_factory_->NewFromAny(names); } -auto ObjectManipulator::Get(v8::Isolate* isolate, - Value* obj_ptr, - Value* key_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); +auto ObjectManipulator::Get(Value* obj_ptr, Value* key_ptr) -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_obj_val = - obj_ptr->ToV8Value(isolate, local_context); + const v8::Local local_obj_val = obj_ptr->Global()->Get(isolate); const v8::Local local_obj = local_obj_val.As(); - const v8::Local local_key = - key_ptr->ToV8Value(isolate, local_context); + const v8::Local local_key = key_ptr->Global()->Get(isolate); - if (!local_obj->Has(local_context, local_key).ToChecked()) { - return val_factory_->New("No such key", type_key_exception); + const v8::Local context = isolate_manager_->GetLocalContext(); + + if (!local_obj->Has(context, local_key).ToChecked()) { + return val_factory_->NewFromString("No such key", type_key_exception); } const v8::Local value = - local_obj->Get(local_context, local_key).ToLocalChecked(); + local_obj->Get(context, local_key).ToLocalChecked(); - return val_factory_->New(local_context, value); + return val_factory_->NewFromAny(value); } -auto ObjectManipulator::Set(v8::Isolate* isolate, - Value* obj_ptr, +auto ObjectManipulator::Set(Value* obj_ptr, Value* key_ptr, Value* val_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_obj_val = - obj_ptr->ToV8Value(isolate, local_context); + const v8::Local local_obj_val = obj_ptr->Global()->Get(isolate); const v8::Local local_obj = local_obj_val.As(); - const v8::Local local_key = - key_ptr->ToV8Value(isolate, local_context); - const v8::Local local_value = - val_ptr->ToV8Value(isolate, local_context); + const v8::Local local_key = key_ptr->Global()->Get(isolate); + const v8::Local local_value = val_ptr->Global()->Get(isolate); + + const v8::Local context = isolate_manager_->GetLocalContext(); - local_obj->Set(local_context, local_key, local_value).ToChecked(); + local_obj->Set(context, local_key, local_value).ToChecked(); - return val_factory_->New(true); + return val_factory_->NewFromBool(true); } -auto ObjectManipulator::Del(v8::Isolate* isolate, - Value* obj_ptr, - Value* key_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); +auto ObjectManipulator::Del(Value* obj_ptr, Value* key_ptr) -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_obj_val = - obj_ptr->ToV8Value(isolate, local_context); + const v8::Local local_obj_val = obj_ptr->Global()->Get(isolate); const v8::Local local_obj = local_obj_val.As(); - const v8::Local local_key = - key_ptr->ToV8Value(isolate, local_context); + const v8::Local local_key = key_ptr->Global()->Get(isolate); - if (!local_obj->Has(local_context, local_key).ToChecked()) { - return val_factory_->New("No such key", type_key_exception); + const v8::Local context = isolate_manager_->GetLocalContext(); + + if (!local_obj->Has(context, local_key).ToChecked()) { + return val_factory_->NewFromString("No such key", type_key_exception); } - return val_factory_->New( - local_obj->Delete(local_context, local_key).ToChecked()); + return val_factory_->NewFromBool( + local_obj->Delete(context, local_key).ToChecked()); } -auto ObjectManipulator::Splice(v8::Isolate* isolate, - Value* obj_ptr, +auto ObjectManipulator::Splice(Value* obj_ptr, int32_t start, int32_t delete_count, Value* new_val_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_obj_val = - obj_ptr->ToV8Value(isolate, local_context); + const v8::Local local_obj_val = obj_ptr->Global()->Get(isolate); const v8::Local local_obj = local_obj_val.As(); // Array.prototype.splice doesn't exist in C++ in V8. We have to find the JS @@ -139,15 +107,17 @@ auto ObjectManipulator::Splice(v8::Isolate* isolate, const v8::Local splice_name = v8::String::NewFromUtf8Literal(isolate, "splice"); + const v8::Local context = isolate_manager_->GetLocalContext(); + v8::Local splice_val; - if (!local_obj->Get(local_context, splice_name).ToLocal(&splice_val)) { - return val_factory_->New("no splice method on object", - type_execute_exception); + if (!local_obj->Get(context, splice_name).ToLocal(&splice_val)) { + return val_factory_->NewFromString("no splice method on object", + type_execute_exception); } if (!splice_val->IsFunction()) { - return val_factory_->New("splice member is not a function", - type_execute_exception); + return val_factory_->NewFromString("splice member is not a function", + type_execute_exception); } const v8::Local splice_func = splice_val.As(); @@ -159,29 +129,23 @@ auto ObjectManipulator::Splice(v8::Isolate* isolate, v8::Int32::New(isolate, delete_count), }; if (new_val_ptr != nullptr) { - argv.push_back(new_val_ptr->ToV8Value(isolate, local_context)); + argv.push_back(new_val_ptr->Global()->Get(isolate)); } v8::MaybeLocal maybe_value = splice_func->Call( - local_context, local_obj, static_cast(argv.size()), argv.data()); + context, local_obj, static_cast(argv.size()), argv.data()); if (maybe_value.IsEmpty()) { - return val_factory_->New(local_context, trycatch.Message(), - trycatch.Exception(), type_execute_exception); + return val_factory_->NewFromException( + trycatch.Message(), trycatch.Exception(), type_execute_exception); } - return val_factory_->New(local_context, maybe_value.ToLocalChecked()); + return val_factory_->NewFromAny(maybe_value.ToLocalChecked()); } -auto ObjectManipulator::Push(v8::Isolate* isolate, - Value* obj_ptr, - Value* new_val_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); +auto ObjectManipulator::Push(Value* obj_ptr, Value* new_val_ptr) -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_obj_val = - obj_ptr->ToV8Value(isolate, local_context); + const v8::Local local_obj_val = obj_ptr->Global()->Get(isolate); const v8::Local local_obj = local_obj_val.As(); // Array.prototype.push doesn't exist in C++ in V8. We have to find the JS @@ -189,15 +153,17 @@ auto ObjectManipulator::Push(v8::Isolate* isolate, const v8::Local push_name = v8::String::NewFromUtf8Literal(isolate, "push"); + const v8::Local context = isolate_manager_->GetLocalContext(); + v8::Local push_val; - if (!local_obj->Get(local_context, push_name).ToLocal(&push_val)) { - return val_factory_->New("no push method on object", - type_execute_exception); + if (!local_obj->Get(context, push_name).ToLocal(&push_val)) { + return val_factory_->NewFromString("no push method on object", + type_execute_exception); } if (!push_val->IsFunction()) { - return val_factory_->New("push member is not a function", - type_execute_exception); + return val_factory_->NewFromString("push member is not a function", + type_execute_exception); } const v8::Local push_func = push_val.As(); @@ -205,33 +171,28 @@ auto ObjectManipulator::Push(v8::Isolate* isolate, const v8::TryCatch trycatch(isolate); std::vector> argv = { - new_val_ptr->ToV8Value(isolate, local_context)}; + new_val_ptr->Global()->Get(isolate)}; v8::MaybeLocal maybe_value = push_func->Call( - local_context, local_obj, static_cast(argv.size()), argv.data()); + context, local_obj, static_cast(argv.size()), argv.data()); if (maybe_value.IsEmpty()) { - return val_factory_->New(local_context, trycatch.Message(), - trycatch.Exception(), type_execute_exception); + return val_factory_->NewFromException( + trycatch.Message(), trycatch.Exception(), type_execute_exception); } - return val_factory_->New(local_context, maybe_value.ToLocalChecked()); + return val_factory_->NewFromAny(maybe_value.ToLocalChecked()); } -auto ObjectManipulator::Call(v8::Isolate* isolate, - Value* func_ptr, +auto ObjectManipulator::Call(Value* func_ptr, Value* this_ptr, Value* argv_ptr) -> Value::Ptr { - const v8::Isolate::Scope isolate_scope(isolate); - const v8::HandleScope handle_scope(isolate); - const v8::Local local_context = context_->Get()->Get(isolate); - const v8::Context::Scope context_scope(local_context); + v8::Isolate* isolate = isolate_manager_->GetIsolate(); - const v8::Local local_func_val = - func_ptr->ToV8Value(isolate, local_context); + const v8::Local local_func_val = func_ptr->Global()->Get(isolate); if (!local_func_val->IsFunction()) { - return val_factory_->New("function is not callable", - type_execute_exception); + return val_factory_->NewFromString("function is not callable", + type_execute_exception); } const v8::Local local_func = local_func_val.As(); @@ -240,34 +201,37 @@ auto ObjectManipulator::Call(v8::Isolate* isolate, if (this_ptr == nullptr) { local_this = v8::Undefined(isolate); } else { - local_this = this_ptr->ToV8Value(isolate, local_context); + local_this = this_ptr->Global()->Get(isolate); } const v8::Local local_argv_value = - argv_ptr->ToV8Value(isolate, local_context); + argv_ptr->Global()->Get(isolate); if (!local_argv_value->IsArray()) { - return val_factory_->New("argv is not an array", type_execute_exception); + return val_factory_->NewFromString("argv is not an array", + type_execute_exception); } const v8::Local local_argv_array = local_argv_value.As(); + const v8::Local context = isolate_manager_->GetLocalContext(); + std::vector> argv; for (uint32_t i = 0; i < local_argv_array->Length(); i++) { - argv.push_back(local_argv_array->Get(local_context, i).ToLocalChecked()); + argv.push_back(local_argv_array->Get(context, i).ToLocalChecked()); } const v8::TryCatch trycatch(isolate); v8::MaybeLocal maybe_value = local_func->Call( - local_context, local_this, static_cast(argv.size()), argv.data()); + context, local_this, static_cast(argv.size()), argv.data()); if (maybe_value.IsEmpty()) { - return val_factory_->New(local_context, trycatch.Message(), - trycatch.Exception(), type_execute_exception); + return val_factory_->NewFromException( + trycatch.Message(), trycatch.Exception(), type_execute_exception); } - return val_factory_->New(local_context, maybe_value.ToLocalChecked()); + return val_factory_->NewFromAny(maybe_value.ToLocalChecked()); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/object_manipulator.h b/src/v8_py_frontend/object_manipulator.h index 7977c0f9..34d3fa57 100644 --- a/src/v8_py_frontend/object_manipulator.h +++ b/src/v8_py_frontend/object_manipulator.h @@ -1,10 +1,8 @@ #ifndef INCLUDE_MINI_RACER_OBJECT_MANIPULATOR_H #define INCLUDE_MINI_RACER_OBJECT_MANIPULATOR_H -#include -#include #include -#include "context_holder.h" +#include "isolate_manager.h" #include "value.h" namespace MiniRacer { @@ -17,32 +15,23 @@ namespace MiniRacer { * the Value pointers is done by the caller. */ class ObjectManipulator { public: - ObjectManipulator(ContextHolder* context, ValueFactory* val_factory); + ObjectManipulator(IsolateManager* isolate_manager_, + ValueFactory* val_factory); - auto GetIdentityHash(v8::Isolate* isolate, Value* obj_ptr) -> Value::Ptr; - auto GetOwnPropertyNames(v8::Isolate* isolate, - Value* obj_ptr) const -> Value::Ptr; - auto Get(v8::Isolate* isolate, Value* obj_ptr, Value* key_ptr) -> Value::Ptr; - auto Set(v8::Isolate* isolate, - Value* obj_ptr, - Value* key_ptr, - Value* val_ptr) -> Value::Ptr; - auto Del(v8::Isolate* isolate, Value* obj_ptr, Value* key_ptr) -> Value::Ptr; - auto Splice(v8::Isolate* isolate, - Value* obj_ptr, + auto GetIdentityHash(Value* obj_ptr) -> Value::Ptr; + auto GetOwnPropertyNames(Value* obj_ptr) const -> Value::Ptr; + auto Get(Value* obj_ptr, Value* key_ptr) -> Value::Ptr; + auto Set(Value* obj_ptr, Value* key_ptr, Value* val_ptr) -> Value::Ptr; + auto Del(Value* obj_ptr, Value* key_ptr) -> Value::Ptr; + auto Splice(Value* obj_ptr, int32_t start, int32_t delete_count, Value* new_val_ptr) -> Value::Ptr; - auto Push(v8::Isolate* isolate, - Value* obj_ptr, - Value* new_val_ptr) -> Value::Ptr; - auto Call(v8::Isolate* isolate, - Value* func_ptr, - Value* this_ptr, - Value* argv_ptr) -> Value::Ptr; + auto Push(Value* obj_ptr, Value* new_val_ptr) -> Value::Ptr; + auto Call(Value* func_ptr, Value* this_ptr, Value* argv_ptr) -> Value::Ptr; private: - ContextHolder* context_; + IsolateManager* isolate_manager_; ValueFactory* val_factory_; }; diff --git a/src/v8_py_frontend/value.cc b/src/v8_py_frontend/value.cc index 9b371d1c..2db6671b 100644 --- a/src/v8_py_frontend/value.cc +++ b/src/v8_py_frontend/value.cc @@ -12,129 +12,183 @@ #include #include #include -#include #include -#include #include #include -#include -#include -#include "isolate_object_collector.h" +#include "isolate_manager.h" namespace MiniRacer { // NOLINTBEGIN(cppcoreguidelines-pro-type-union-access) -Value::Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, - v8::Local context, - v8::Local value) - : isolate_object_collector_(isolate_object_collector) { +namespace { +auto InferTypeFromValue(v8::Local value) -> ValueTypes { if (value->IsNull()) { - handle_.type = type_null; - } else if (value->IsUndefined()) { - handle_.type = type_undefined; - } else if (value->IsInt32()) { - handle_.type = type_integer; - auto val = value->Int32Value(context).ToChecked(); - handle_.int_val = val; + return type_null; + } + if (value->IsUndefined()) { + return type_undefined; + } + if (value->IsFunction()) { + return type_function; + } + if (value->IsSymbol()) { + return type_symbol; + } + if (value->IsPromise()) { + return type_promise; + } + if (value->IsArray()) { + return type_array; + } + if (value->IsInt32() || value->IsBigInt()) { + return type_integer; + } + if (value->IsNumber()) { + return type_double; + } + if (value->IsBoolean()) { + return type_bool; + } + if (value->IsDate()) { + return type_date; + } + if (value->IsString()) { + return type_string; } - // ECMA-262, 4.3.20 - // http://www.ecma-international.org/ecma-262/5.1/#sec-4.3.19 - else if (value->IsNumber()) { - handle_.type = type_double; - const double val = value->NumberValue(context).ToChecked(); - handle_.double_val = val; + if (value->IsArrayBufferView()) { + return type_array_buffer_view; + } + if (value->IsSharedArrayBuffer()) { + return type_shared_array_buffer; + } + if (value->IsArrayBuffer()) { + return type_array_buffer; + } + if (value->IsObject()) { + return type_object; + } + + return type_invalid; +} +} // namespace + +Value::Value(v8::Isolate* isolate, + v8::Local context, + v8::Local value, + ValueTypes type) + : global_(isolate, value) { + handle_.type = type; + + // For various types we store a "preview" so that the user can read data + // without another trip through the FFI: + if (value->IsInt32()) { + handle_.int_val = value->Int32Value(context).ToChecked(); + } else if (value->IsBigInt()) { + handle_.int_val = value.As()->Int64Value(); + } else if (value->IsNumber()) { + handle_.double_val = value->NumberValue(context).ToChecked(); } else if (value->IsBoolean()) { - handle_.type = type_bool; handle_.int_val = (value->IsTrue() ? 1 : 0); - } else if (value->IsFunction()) { - handle_.type = type_function; - SaveGlobalHandle(isolate, value); - } else if (value->IsSymbol()) { - handle_.type = type_symbol; - SaveGlobalHandle(isolate, value); } else if (value->IsDate()) { - handle_.type = type_date; - const v8::Local date = v8::Local::Cast(value); - - const double timestamp = date->ValueOf(); - handle_.double_val = timestamp; + handle_.double_val = v8::Local::Cast(value)->ValueOf(); } else if (value->IsString()) { const v8::Local rstr = value->ToString(context).ToLocalChecked(); - handle_.type = type_str_utf8; handle_.len = static_cast(rstr->Utf8LengthV2(isolate)); // in bytes const size_t capacity = handle_.len + 1; - auto& msg = data_.emplace>(); - msg.resize(capacity); - rstr->WriteUtf8V2(isolate, msg.data(), capacity); - handle_.bytes = msg.data(); - } else if (value->IsSharedArrayBuffer() || value->IsArrayBuffer() || - value->IsArrayBufferView()) { - CreateBackingStoreRef(isolate, value); - } else if (value->IsPromise()) { - handle_.type = type_promise; - SaveGlobalHandle(isolate, value); - } else if (value->IsArray()) { - handle_.type = type_array; - SaveGlobalHandle(isolate, value); - } else if (value->IsObject()) { - handle_.type = type_object; - SaveGlobalHandle(isolate, value); + buf_.resize(capacity); + rstr->WriteUtf8V2(isolate, buf_.data(), capacity); + handle_.bytes = buf_.data(); + } else if (value->IsArrayBufferView()) { + // For ArrayBuffer and friends, we store a reference to the V8 object + // in this Value instance, and return a pointer *into* the buffer to the + // Python side. + const v8::Local view = + v8::Local::Cast(value); + + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) + handle_.bytes = + static_cast(view->Buffer()->GetBackingStore()->Data()) + + view->ByteOffset(); + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) + handle_.len = view->ByteLength(); + } else if (value->IsSharedArrayBuffer()) { + auto backing_store = + v8::Local::Cast(value)->GetBackingStore(); + handle_.bytes = static_cast(backing_store->Data()); + handle_.len = backing_store->ByteLength(); + } else if (value->IsArrayBuffer()) { + auto backing_store = + v8::Local::Cast(value)->GetBackingStore(); + handle_.bytes = static_cast(backing_store->Data()); + handle_.len = backing_store->ByteLength(); } } -Value::Value(v8::Isolate* /*isolate*/, - IsolateObjectCollector* isolate_object_collector, - std::string_view val, - ValueTypes type) - : isolate_object_collector_(isolate_object_collector) { - handle_.len = val.size(); - handle_.type = type; - auto& msg = data_.emplace>(); - msg.resize(handle_.len + 1); - std::copy(val.begin(), val.end(), msg.data()); - msg[handle_.len] = '\0'; - handle_.bytes = msg.data(); +auto Value::Global() -> v8::Global* { + return &global_; } -Value::Value(v8::Isolate* /*isolate*/, - IsolateObjectCollector* isolate_object_collector, - bool val) - : isolate_object_collector_(isolate_object_collector) { - handle_.len = 0; - handle_.type = type_bool; - handle_.int_val = val ? 1 : 0; +auto Value::GetHandle() -> ValueHandle* { + return &handle_; } -Value::Value(v8::Isolate* /*isolate*/, - IsolateObjectCollector* isolate_object_collector, - int64_t val, - ValueTypes type) - : isolate_object_collector_(isolate_object_collector) { - handle_.len = 0; - handle_.type = type; - handle_.int_val = val; +// NOLINTEND(cppcoreguidelines-pro-type-union-access) + +ValueFactory::ValueFactory(IsolateManager* isolate_manager) + : isolate_manager_(isolate_manager) {} + +auto ValueFactory::New(v8::Local value, + ValueTypes type) -> Value::Ptr { + const v8::Local context = isolate_manager_->GetLocalContext(); + return std::make_unique(isolate_manager_->GetIsolate(), context, value, + type); } -Value::Value(v8::Isolate* /*isolate*/, - IsolateObjectCollector* isolate_object_collector, - double val, - ValueTypes type) - : isolate_object_collector_(isolate_object_collector) { - handle_.len = 0; - handle_.type = type; - handle_.double_val = val; +auto ValueFactory::NewFromAny(v8::Local value) -> Value::Ptr { + return New(value, InferTypeFromValue(value)); } -namespace { -// From v8/src/d8.cc: -auto ExceptionToString(v8::Isolate* isolate, - v8::Local context, - v8::Local message, - v8::Local exception_obj) -> std::string { +auto ValueFactory::NewFromBool(bool value) -> Value::Ptr { + return New(v8::Boolean::New(isolate_manager_->GetIsolate(), value), + type_bool); +} + +auto ValueFactory::NewFromInt(int64_t value, ValueTypes type) -> Value::Ptr { + v8::Isolate* isolate = isolate_manager_->GetIsolate(); + if (type == type_undefined) { + return New(v8::Undefined(isolate), type); + } + if (type == type_null) { + return New(v8::Null(isolate), type); + } + return New(v8::BigInt::New(isolate, value), type); +} + +auto ValueFactory::NewFromDouble(double value, ValueTypes type) -> Value::Ptr { + if (type == type_date) { + return New(v8::Date::New(isolate_manager_->GetLocalContext(), value) + .ToLocalChecked(), + type); + } + return New(v8::Number::New(isolate_manager_->GetIsolate(), value), type); +} + +auto ValueFactory::NewFromString(std::string_view message, + ValueTypes type) -> Value::Ptr { + return New(v8::String::NewFromUtf8(isolate_manager_->GetIsolate(), + message.data(), v8::NewStringType::kNormal, + static_cast(message.size())) + .ToLocalChecked(), + type); +} + +auto ValueFactory::NewFromException(v8::Local message, + v8::Local exception_obj, + ValueTypes type) -> Value::Ptr { + // From v8/src/d8.cc: std::stringstream msg; // Converts a V8 value to a C string. @@ -142,7 +196,10 @@ auto ExceptionToString(v8::Isolate* isolate, return (*value == nullptr) ? "" : *value; }; - const v8::String::Utf8Value exception(isolate, exception_obj); + const v8::Local context = isolate_manager_->GetLocalContext(); + + const v8::String::Utf8Value exception(isolate_manager_->GetIsolate(), + exception_obj); const char* exception_string = ToCString(exception); if (message.IsEmpty()) { // V8 didn't provide any extra information about this error; just @@ -157,7 +214,8 @@ auto ExceptionToString(v8::Isolate* isolate, } else { // Print (filename):(line number): (message). const v8::String::Utf8Value filename( - isolate, message->GetScriptOrigin().ResourceName()); + isolate_manager_->GetIsolate(), + message->GetScriptOrigin().ResourceName()); const char* filename_string = ToCString(filename); const int linenum = message->GetLineNumber(context).FromMaybe(-1); msg << filename_string << ":" << linenum << ": " << exception_string @@ -165,7 +223,8 @@ auto ExceptionToString(v8::Isolate* isolate, v8::Local sourceline; if (message->GetSourceLine(context).ToLocal(&sourceline)) { // Print line of source code. - const v8::String::Utf8Value sourcelinevalue(isolate, sourceline); + const v8::String::Utf8Value sourcelinevalue( + isolate_manager_->GetIsolate(), sourceline); const char* sourceline_string = ToCString(sourcelinevalue); msg << sourceline_string << "\n"; // Print wavy underline (GetUnderline is deprecated). @@ -185,155 +244,35 @@ auto ExceptionToString(v8::Isolate* isolate, .ToLocal(&stack_trace_string) && stack_trace_string->IsString()) { const v8::String::Utf8Value stack_trace( - isolate, stack_trace_string.As()); + isolate_manager_->GetIsolate(), stack_trace_string.As()); msg << "\n"; msg << ToCString(stack_trace); msg << "\n"; } - return msg.str(); -} -} // end anonymous namespace - -Value::Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, - v8::Local context, - v8::Local message, - v8::Local exception_obj, - ValueTypes result_type) - : Value(isolate, - isolate_object_collector, - ExceptionToString(isolate, context, message, exception_obj), - result_type) {} - -Value::~Value() { - if (std::holds_alternative>(data_)) { - isolate_object_collector_->Collect( - std::get>(std::move(data_))); - } -} - -auto Value::ToV8Value(v8::Isolate* isolate, - v8::Local context) -> v8::Local { - // If we've saved a handle to a v8::Global, we can return the exact v8 - // value to which this Value refers: - auto* global_handle = std::get_if>(&data_); - if (global_handle != nullptr) { - return global_handle->Get(isolate); - } - - // Otherwise, try and rehydrate a v8::Value based on data stored in the - // ValueHandle: - - if (handle_.type == type_null) { - return v8::Null(isolate); - } - - if (handle_.type == type_undefined) { - return v8::Undefined(isolate); - } - - if (handle_.type == type_integer) { - return v8::Integer::New(isolate, static_cast(handle_.int_val)); - } - - if (handle_.type == type_double) { - return v8::Number::New(isolate, handle_.double_val); - } - - if (handle_.type == type_bool) { - return v8::Boolean::New(isolate, handle_.int_val != 0); - } - - if (handle_.type == type_date) { - return v8::Date::New(context, handle_.double_val).ToLocalChecked(); - } - - if (handle_.type == type_str_utf8) { - return v8::String::NewFromUtf8(isolate, handle_.bytes, - v8::NewStringType::kNormal, - static_cast(handle_.len)) - .ToLocalChecked(); - } - - // Unknown type! - return v8::Undefined(isolate); -} - -auto Value::GetHandle() -> ValueHandle* { - return &handle_; -} - -void Value::SaveGlobalHandle(v8::Isolate* isolate, v8::Local value) { - data_.emplace>(isolate, value); -} - -void Value::CreateBackingStoreRef(v8::Isolate* isolate, - v8::Local value) { - // For ArrayBuffer and friends, we store a reference to the V8 object - // in this Value instance, and return a pointer *into* the buffer to the - // Python side. - - size_t offset = 0; - size_t size = 0; - std::shared_ptr backing_store; - - if (value->IsArrayBufferView()) { - const v8::Local view = - v8::Local::Cast(value); - - backing_store = view->Buffer()->GetBackingStore(); - offset = view->ByteOffset(); - size = view->ByteLength(); - } else if (value->IsSharedArrayBuffer()) { - backing_store = - v8::Local::Cast(value)->GetBackingStore(); - size = backing_store->ByteLength(); - } else { - backing_store = v8::Local::Cast(value)->GetBackingStore(); - size = backing_store->ByteLength(); - } - - handle_.type = value->IsSharedArrayBuffer() ? type_shared_array_buffer - : type_array_buffer; - // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) - handle_.bytes = static_cast(backing_store->Data()) + offset; - // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) - handle_.len = size; - - SaveGlobalHandle(isolate, value); + return NewFromString(msg.str(), type); } -// NOLINTEND(cppcoreguidelines-pro-type-union-access) - -ValueFactory::ValueFactory(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector) - : isolate_(isolate), isolate_object_collector_(isolate_object_collector) {} - auto ValueRegistry::Remember(Value::Ptr ptr) -> ValueHandle* { - const std::lock_guard lock(mutex_); ValueHandle* handle = ptr->GetHandle(); values_[handle] = std::move(ptr); return handle; } void ValueRegistry::Forget(ValueHandle* handle) { - const std::lock_guard lock(mutex_); values_.erase(handle); } -auto ValueRegistry::FromHandle(ValueHandle* handle) -> Value::Ptr { +auto ValueRegistry::FromHandle(ValueHandle* handle) -> Value* { // Track all created values to relieve Python of the duty of garbage // collecting them in the correct order relative to the MiniRacer::Context: - const std::lock_guard lock(mutex_); auto iter = values_.find(handle); if (iter == values_.end()) { - return {}; + return nullptr; } - return iter->second; + return iter->second.get(); } auto ValueRegistry::Count() -> size_t { - const std::lock_guard lock(mutex_); return values_.size(); } diff --git a/src/v8_py_frontend/value.h b/src/v8_py_frontend/value.h index acbf5231..2f4a1039 100644 --- a/src/v8_py_frontend/value.h +++ b/src/v8_py_frontend/value.h @@ -9,12 +9,10 @@ #include #include #include -#include #include #include -#include #include -#include "isolate_object_collector.h" +#include "isolate_manager.h" namespace MiniRacer { @@ -24,7 +22,7 @@ enum ValueTypes : uint8_t { type_bool = 2, type_integer = 3, type_double = 4, - type_str_utf8 = 5, + type_string = 5, type_array = 6, // type_hash = 7, // deprecated type_date = 8, @@ -35,7 +33,8 @@ enum ValueTypes : uint8_t { type_function = 100, type_shared_array_buffer = 101, type_array_buffer = 102, - type_promise = 103, + type_array_buffer_view = 103, + type_promise = 104, type_execute_exception = 200, type_parse_exception = 201, @@ -71,42 +70,13 @@ struct ValueHandle { class Value { public: Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, - std::string_view val, - ValueTypes result_type); - Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, - bool val); - Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, - int64_t val, - ValueTypes result_type); - Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, - double val, - ValueTypes result_type); - Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, v8::Local context, - v8::Local value); - Value(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector, - v8::Local context, - v8::Local message, - v8::Local exception_obj, - ValueTypes result_type); - - ~Value(); - - Value(const Value& other) = delete; - Value(Value&& other) noexcept = delete; - auto operator=(const Value& other) -> Value& = delete; - auto operator=(Value&& other) noexcept -> Value& = delete; + v8::Local value, + ValueTypes type); - using Ptr = std::shared_ptr; + using Ptr = std::unique_ptr; - auto ToV8Value(v8::Isolate* isolate, - v8::Local context) -> v8::Local; + auto Global() -> v8::Global*; friend class ValueRegistry; @@ -115,22 +85,33 @@ class Value { void SaveGlobalHandle(v8::Isolate* isolate, v8::Local value); void CreateBackingStoreRef(v8::Isolate* isolate, v8::Local value); - IsolateObjectCollector* isolate_object_collector_; ValueHandle handle_; - std::variant, v8::Global> data_; + v8::Global global_; + std::vector buf_; }; class ValueFactory { public: - explicit ValueFactory(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector); + explicit ValueFactory(IsolateManager* isolate_manager); - template - auto New(Params&&... params) -> Value::Ptr; + auto New(v8::Local value, ValueTypes type) -> Value::Ptr; + + auto NewFromAny(v8::Local value) -> Value::Ptr; + + auto NewFromBool(bool value) -> Value::Ptr; + + auto NewFromInt(int64_t value, ValueTypes type) -> Value::Ptr; + + auto NewFromDouble(double value, ValueTypes type) -> Value::Ptr; + + auto NewFromString(std::string_view message, ValueTypes type) -> Value::Ptr; + + auto NewFromException(v8::Local message, + v8::Local exception_obj, + ValueTypes type) -> Value::Ptr; private: - v8::Isolate* isolate_; - IsolateObjectCollector* isolate_object_collector_; + IsolateManager* isolate_manager_; }; /** We return handles to Values to the MiniRacer user side (i.e., @@ -152,22 +133,15 @@ class ValueRegistry { /** "Re-hydrate" a value from just its handle (only works if it was * "Remembered") */ - auto FromHandle(ValueHandle* handle) -> Value::Ptr; + auto FromHandle(ValueHandle* handle) -> Value*; /** Count the total number of remembered values, for test purposes. */ auto Count() -> size_t; private: - std::mutex mutex_; - std::unordered_map> values_; + std::unordered_map values_; }; -template -inline auto ValueFactory::New(Params&&... params) -> Value::Ptr { - return std::make_shared(isolate_, isolate_object_collector_, - std::forward(params)...); -} - } // namespace MiniRacer #endif // INCLUDE_MINI_RACER_VALUE_H diff --git a/tests/test_eval.py b/tests/test_eval.py index d5bebee7..1216e5c9 100644 --- a/tests/test_eval.py +++ b/tests/test_eval.py @@ -179,6 +179,7 @@ def test_null_byte() -> None: result = mr.eval(in_val) assert result == s + del result assert_no_v8_objects(mr) diff --git a/tests/test_objects.py b/tests/test_objects.py index 9363e642..f08df780 100644 --- a/tests/test_objects.py +++ b/tests/test_objects.py @@ -9,6 +9,7 @@ JSFunction, JSObject, JSPromise, + JSString, JSSymbol, JSUndefined, MiniRacer, @@ -56,7 +57,12 @@ def test_object_read() -> None: ("null", "null_value"), ("undefined", "undef_value"), ] - assert set(obj.values()) == {42, "key_is_number", "undef_value", "null_value"} + assert {str(s) if isinstance(s, JSString) else s for s in obj.values()} == { + 42, + "key_is_number", + "undef_value", + "null_value", + } obj2 = mr.eval( """\ var a = { diff --git a/tests/test_types.py b/tests/test_types.py index 99a68f02..b7f216ff 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -15,6 +15,7 @@ JSEvalException, JSFunction, JSObject, + JSString, JSSymbol, JSUndefined, MiniRacer, @@ -65,6 +66,15 @@ def test_str() -> None: v("string with null \0 byte") +def test_str_is_jsstring() -> None: + mr = MiniRacer() + s = mr.eval("'foo'") + assert isinstance(s, JSString) + + del s + assert_no_v8_objects(mr) + + def test_unicode() -> None: ustr = "\N{GREEK CAPITAL LETTER DELTA}" mr = MiniRacer() @@ -72,6 +82,7 @@ def test_unicode() -> None: assert ustr == res _test_round_trip(mr, ustr) + del res assert_no_v8_objects(mr)