diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index fcb05209..c50bfa07 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -399,25 +399,25 @@ have for JavaScript. Wherever possible, we avoid interchanging raw pointers betw and Python. Instead, we interchange integer IDs. The C++ side of PyMiniRacer can convert integer IDs to raw pointers using a map, after validating that the IDs are still valid. -### ... except for `BinaryValueHandle` pointers +### ... except for `ValueHandle` pointers -We break the above rule for `BinaryValueHandle` pointers. PyMiniRacer uses -`BinaryValueHandle` to exchange most data between Python and C++. Python directly reads -the contents of `BinaryValueHandle` pointers, to read primitive values (e.g., booleans, -integers, and strings). +We break the above rule for `ValueHandle` pointers. PyMiniRacer uses `ValueHandle` to +exchange most data between Python and C++. Python directly reads the contents of +`ValueHandle` pointers, to read primitive values (e.g., booleans, integers, and +strings). We do this for theoretical performance reasons which have not yet been validated. To be consistent with the rest of PyMiniRacer's design, we _could_ create an API like: -1. C++ generates a numeric `value_id` and stores a BinaryValue in a - `std::unordered_map>`. +1. C++ generates a numeric `value_id` and stores a Value in a + `std::unordered_map>`. 1. C++ gives Python that `value_id` to Python. 1. To get any data Python has to call APIs like `mr_value_type(context_id, value_id)`, `mr_value_as_bool(context_id, value_id)`, `mr_value_as_string_len(context_id, value_id)`, `mr_value_as_string(context_id, value_id, buf, buflen)`, ... 1. Eventually Python calls `mr_value_free(context_id, value_id)` which wipes out the map - entry, thus freeing the `BinaryValue`. + entry, thus freeing the `Value`. _\*\*Note: We don't do this. The above is \_not_ how PyMiniRacer actually handles values.\*\*\_ @@ -428,11 +428,11 @@ would be nice to switch to that model if it's sufficiently performant. For now at least, we instead use raw pointers for this case. -We still don't fully trust Python with the lifecyce of `BinaryValueHandle` pointers; -when Python passes these pointers back to C++, we still check validity by looking up the +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 `BinaryValue` object). The C++ `MiniRacer::BinaryValueFactory` can -authoritatively destruct any dangling `BinaryValue` objects when it exits. +of the `Value` object). The C++ `MiniRacer::ValueFactory` can authoritatively destruct +any dangling `Value` objects when it exits. This last especially helps with an odd scenario introduced by Python `__del__`: the order in which Python calls `__del__` on a collection of objects is neither guaranteed @@ -440,18 +440,18 @@ nor very predictable. When a Python program drops references to a Python `MiniRa object, it's common for Python to call `_Context.__del__` before it calls `ValHandle.__del__`, thus destroying _the container for_ the value before it destroys the value itself. The C++ side of PyMiniRacer can easily detect this scenario: First, -when destroying the `MiniRacer::Context`, it sees straggling `BinaryValue`s and destroys -them. Then, when Python asks C++ to destroy the straggling `BinaryValueHandle`s, the C++ +when destroying the `MiniRacer::Context`, it sees straggling `Value`s and destroys them. +Then, when Python asks C++ to destroy the straggling `ValueHandle`s, the C++ `mr_free_value` API sees the `MiniRacer::Context` is already gone, and ignores the redundant request. The above scenario does imply a possibility for dangling pointer access: if Python calls -`_Context.__del__` then tries to read the memory addressed by the raw -`BinaryValueHandle` pointers, it will be committing a use-after-free error. We mitigate -this problem by hiding `BinaryValueHandle` within PyMiniRacer's Python code, and by -giving `ValHandle` (our Python wrapper of `BinaryValueHandle`) a reference to the -`_Context`, preventing the context from being finalized until the `ValHandle` is _also_ -in Python's garbage list and on its way out. +`_Context.__del__` then tries to read the memory addressed by the raw `ValueHandle` +pointers, it will be committing a use-after-free error. We mitigate this problem by +hiding `ValueHandle` within PyMiniRacer's Python code, and by giving `ValHandle` (our +Python wrapper of `ValueHandle`) a reference to the `_Context`, preventing the context +from being finalized until the `ValHandle` is _also_ in Python's garbage list and on its +way out. ### Only touch (most of) the `v8::Isolate` from within the message loop @@ -485,10 +485,10 @@ an easy API to submit tasks, whose callbacks accept as their first-and-only argu saving a copy of the pointer and using it later would defeat the point; don't do that.) One odd tidbit of PyMiniRacer is that _even object destruction_ has to use the above -pattern. For example, it is (probably) not safe to free a `v8::Persistent` without -holding the isolate lock, so when a non-message-loop thread needs to destroy a wrapped -V8 value, we enqueue a pretty trivial task for the message loop: -`isolate_manager->Run([persistent]() { delete persistent; })`. +pattern. For example, it is (probably) not safe to free a `v8::Global` without holding +the isolate lock, so when a non-message-loop thread needs to destroy a wrapped V8 value, +we enqueue a pretty trivial task for the message loop: +`isolate_manager->Run([global]() { delete global; })`. See [here](https://groups.google.com/g/v8-users/c/glG3-3pufCo) for some discussion of this design on the v8-users mailing list. diff --git a/HISTORY.md b/HISTORY.md index 4f183b8f..d6fdabed 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,13 @@ # History +## 0.14.1 (2026-01-31) + +- Fix memory leak when tracking JS objects in Python. We were never calling + v8::Persistent::Reset(). Now we use v8::Global which has no such requirement. +- In the C++ implementation, rename BinaryValue to just Value and optimize things a bit + by collapsing various data members into a Variant. +- Upgrade to V8 14.4 from 14.3. + ## 0.14.0 (2026-01-03) - Major revamp of Python-side async handling: `PyMiniRacer` now manages most @@ -107,7 +115,7 @@ calling the function from Python, e.g., `mr.eval("myfunc")()`. - Hardening (meaning "fixing potential but not-yet-seen bugs") related to freeing - `BinaryValue` instances (which convey data from C++ to Python). + `Value` instances (which convey data from C++ to Python). - More hardening related to race conditions on teardown of the `MiniRacer` object in the unlikely condition that `eval` operations are still executing on the C++ side, and diff --git a/builder/v8_build.py b/builder/v8_build.py index 4800ac56..090bb8c0 100644 --- a/builder/v8_build.py +++ b/builder/v8_build.py @@ -24,7 +24,7 @@ LOGGER = getLogger(__name__) LOGGER.setLevel(DEBUG) ROOT_DIR = Path(__file__).absolute().parents[1] -V8_VERSION = "branch-heads/14.3" +V8_VERSION = "branch-heads/14.4" @cache diff --git a/pyproject.toml b/pyproject.toml index 4ab76dad..1d825eb8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ requires = ["setuptools>=80.9"] [project] name = "mini-racer" -version = "0.14.0" +version = "0.14.1" dynamic = ["readme"] description = "Minimal, modern embedded V8 for Python." license = "ISC" @@ -66,7 +66,6 @@ dev = [ "packaging>=25.0", "pytest>=9.0.2", "ruff>=0.14.8", - "rust-just>=1.43.1", "setuptools[core]>=80.9.0", "types-setuptools>=80.9.0.20250822", ] diff --git a/src/py_mini_racer/_objects.py b/src/py_mini_racer/_objects.py index ff1e69ed..64ee64c9 100644 --- a/src/py_mini_racer/_objects.py +++ b/src/py_mini_racer/_objects.py @@ -259,14 +259,14 @@ class ObjectFactoryImpl: def value_handle_to_python( # noqa: C901, PLR0911, PLR0912 self, ctx: Context, val_handle: ValueHandle ) -> PythonJSConvertedTypes: - """Convert a binary value handle from the C++ side into a Python object.""" + """Convert a value handle from the C++ side into a Python object.""" - # A MiniRacer binary value handle is a pointer to a structure which, for some + # A MiniRacer value handle is a pointer to a structure which, for some # simple types like ints, floats, and strings, is sufficient to describe the # data, enabling us to convert the value immediately and free the handle. # For more complex types, like Objects and Arrays, the handle is just an opaque - # pointer to a V8 object. In these cases, we retain the binary value handle, + # pointer to a V8 object. In these cases, we retain the value handle, # wrapping it in a Python object. We can then use the handle in follow-on API # calls to work with the underlying V8 object. diff --git a/src/v8_py_frontend/BUILD.gn b/src/v8_py_frontend/BUILD.gn index 2a1f381f..9d4b8384 100644 --- a/src/v8_py_frontend/BUILD.gn +++ b/src/v8_py_frontend/BUILD.gn @@ -4,8 +4,8 @@ v8_shared_library("mini_racer") { output_name = "mini_racer" sources = [ "exports.cc", - "binary_value.h", - "binary_value.cc", + "value.h", + "value.cc", "cancelable_task_runner.h", "cancelable_task_runner.cc", "code_evaluator.h", diff --git a/src/v8_py_frontend/binary_value.h b/src/v8_py_frontend/binary_value.h deleted file mode 100644 index d302f2f6..00000000 --- a/src/v8_py_frontend/binary_value.h +++ /dev/null @@ -1,171 +0,0 @@ -#ifndef INCLUDE_MINI_RACER_BINARY_VALUE_H -#define INCLUDE_MINI_RACER_BINARY_VALUE_H - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include "isolate_object_collector.h" - -namespace MiniRacer { - -enum BinaryTypes : uint8_t { - type_invalid = 0, - type_null = 1, - type_bool = 2, - type_integer = 3, - type_double = 4, - type_str_utf8 = 5, - type_array = 6, - // type_hash = 7, // deprecated - type_date = 8, - type_symbol = 9, - type_object = 10, - type_undefined = 11, - - type_function = 100, - type_shared_array_buffer = 101, - type_array_buffer = 102, - type_promise = 103, - - type_execute_exception = 200, - type_parse_exception = 201, - type_oom_exception = 202, - type_timeout_exception = 203, - type_terminated_exception = 204, - type_value_exception = 205, - type_key_exception = 206, -}; - -// NOLINTBEGIN(misc-non-private-member-variables-in-classes) -// NOLINTBEGIN(cppcoreguidelines-owning-memory) -// NOLINTBEGIN(cppcoreguidelines-pro-type-member-init) -// NOLINTBEGIN(hicpp-member-init) -/** A simplified structure designed for sharing data with non-C++ code over a C - * foreign function API (e.g., Python ctypes). This object directly provides - * values for some simple types (e.g., numbers and strings), and also acts as a - * handle for the non-C++ code to manage opaque data via our APIs. */ -struct BinaryValueHandle { - union { - char* bytes; - int64_t int_val; - double double_val; - }; - size_t len; - BinaryTypes type = type_invalid; -} __attribute__((packed)); -// NOLINTEND(hicpp-member-init) -// NOLINTEND(cppcoreguidelines-pro-type-member-init) -// NOLINTEND(cppcoreguidelines-owning-memory) -// NOLINTEND(misc-non-private-member-variables-in-classes) - -class BinaryValue { - public: - BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - std::string_view val, - BinaryTypes result_type); - BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - bool val); - BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - int64_t val, - BinaryTypes result_type); - BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - double val, - BinaryTypes result_type); - BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - v8::Local context, - v8::Local value); - BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - v8::Local context, - v8::Local message, - v8::Local exception_obj, - BinaryTypes result_type); - - using Ptr = std::shared_ptr; - - auto ToValue(v8::Local context) -> v8::Local; - - friend class BinaryValueRegistry; - - private: - auto GetHandle() -> BinaryValueHandle*; - void SavePersistentHandle(v8::Local value); - void CreateBackingStoreRef(v8::Local value); - - v8::Isolate* isolate_; - IsolateObjectDeleter isolate_object_deleter_; - BinaryValueHandle handle_; - std::vector msg_; - std::unique_ptr, IsolateObjectDeleter> - persistent_handle_; - std::unique_ptr, IsolateObjectDeleter> - backing_store_; -}; - -class BinaryValueFactory { - public: - explicit BinaryValueFactory(v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector); - - template - auto New(Params&&... params) -> BinaryValue::Ptr; - - private: - v8::Isolate* isolate_; - IsolateObjectCollector* isolate_object_collector_; -}; - -/** We return handles to BinaryValues to the MiniRacer user side (i.e., - * Python), as raw pointers. To ensure we keep those handles alive while Python - * is using them, we register them in a map, contained within this class. - */ -class BinaryValueRegistry { - public: - /** Record the value in an internal map, so we don't destroy it when - * returning a binary value handle to the MiniRacer user (i.e., the - * Python side). - */ - auto Remember(BinaryValue::Ptr ptr) -> BinaryValueHandle*; - - /** Unrecord a value so it can be garbage collected (once any other - * shared_ptr references are dropped). - */ - void Forget(BinaryValueHandle* handle); - - /** "Re-hydrate" a value from just its handle (only works if it was - * "Remembered") */ - auto FromHandle(BinaryValueHandle* handle) -> BinaryValue::Ptr; - - /** Count the total number of remembered values, for test purposes. */ - auto Count() -> size_t; - - private: - std::mutex mutex_; - std::unordered_map> values_; -}; - -template -inline auto BinaryValueFactory::New(Params&&... params) -> BinaryValue::Ptr { - return std::make_shared( - isolate_, IsolateObjectDeleter(isolate_object_collector_), - std::forward(params)...); -} - -} // namespace MiniRacer - -#endif // INCLUDE_MINI_RACER_BINARY_VALUE_H diff --git a/src/v8_py_frontend/callback.h b/src/v8_py_frontend/callback.h index c42e0773..b28831c2 100644 --- a/src/v8_py_frontend/callback.h +++ b/src/v8_py_frontend/callback.h @@ -3,13 +3,13 @@ #include #include -#include "binary_value.h" +#include "value.h" namespace MiniRacer { -using RawCallback = void (*)(uint64_t, BinaryValueHandle*); +using RawCallback = void (*)(uint64_t, ValueHandle*); -using CallbackFn = std::function; +using CallbackFn = std::function; } // end namespace MiniRacer diff --git a/src/v8_py_frontend/code_evaluator.cc b/src/v8_py_frontend/code_evaluator.cc index cf26b65a..d1f3c2e5 100644 --- a/src/v8_py_frontend/code_evaluator.cc +++ b/src/v8_py_frontend/code_evaluator.cc @@ -9,21 +9,20 @@ #include #include #include -#include "binary_value.h" #include "context_holder.h" #include "isolate_memory_monitor.h" +#include "value.h" namespace MiniRacer { CodeEvaluator::CodeEvaluator(ContextHolder* context, - BinaryValueFactory* bv_factory, + ValueFactory* val_factory, IsolateMemoryMonitor* memory_monitor) : context_(context), - bv_factory_(bv_factory), + val_factory_(val_factory), memory_monitor_(memory_monitor) {} -auto CodeEvaluator::Eval(v8::Isolate* isolate, - BinaryValue* code_ptr) -> BinaryValue::Ptr { +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); @@ -31,10 +30,11 @@ auto CodeEvaluator::Eval(v8::Isolate* isolate, const v8::TryCatch trycatch(isolate); - const v8::Local local_code_val = code_ptr->ToValue(context); + const v8::Local local_code_val = + code_ptr->ToV8Value(isolate, context); if (!local_code_val->IsString()) { - return bv_factory_->New("code is not a string", type_execute_exception); + return val_factory_->New("code is not a string", type_execute_exception); } const v8::Local local_code_str = local_code_val.As(); @@ -47,27 +47,27 @@ auto CodeEvaluator::Eval(v8::Isolate* isolate, if (!v8::Script::Compile(context, local_code_str, &script_origin) .ToLocal(&script) || script.IsEmpty()) { - return bv_factory_->New(context, trycatch.Message(), trycatch.Exception(), - type_parse_exception); + return val_factory_->New(context, trycatch.Message(), trycatch.Exception(), + type_parse_exception); } v8::MaybeLocal maybe_value = script->Run(context); if (!maybe_value.IsEmpty()) { - return bv_factory_->New(context, maybe_value.ToLocalChecked()); + return val_factory_->New(context, maybe_value.ToLocalChecked()); } // Didn't execute. Find an error: if (memory_monitor_->IsHardMemoryLimitReached()) { - return bv_factory_->New("", type_oom_exception); + return val_factory_->New("", type_oom_exception); } - BinaryTypes result_type = type_execute_exception; + ValueTypes result_type = type_execute_exception; if (trycatch.HasTerminated()) { result_type = type_terminated_exception; } - return bv_factory_->New(context, trycatch.Message(), trycatch.Exception(), - result_type); + return val_factory_->New(context, 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 a7669a3e..570ff466 100644 --- a/src/v8_py_frontend/code_evaluator.h +++ b/src/v8_py_frontend/code_evaluator.h @@ -5,9 +5,9 @@ #include #include #include -#include "binary_value.h" #include "context_holder.h" #include "isolate_memory_monitor.h" +#include "value.h" namespace MiniRacer { @@ -15,14 +15,14 @@ namespace MiniRacer { class CodeEvaluator { public: CodeEvaluator(ContextHolder* context, - BinaryValueFactory* bv_factory, + ValueFactory* val_factory, IsolateMemoryMonitor* memory_monitor); - auto Eval(v8::Isolate* isolate, BinaryValue* code_ptr) -> BinaryValue::Ptr; + auto Eval(v8::Isolate* isolate, Value* code_ptr) -> Value::Ptr; private: ContextHolder* context_; - BinaryValueFactory* bv_factory_; + ValueFactory* val_factory_; IsolateMemoryMonitor* memory_monitor_; }; diff --git a/src/v8_py_frontend/context.cc b/src/v8_py_frontend/context.cc index 4fe8c7b9..67654408 100644 --- a/src/v8_py_frontend/context.cc +++ b/src/v8_py_frontend/context.cc @@ -8,7 +8,6 @@ #include #include #include -#include "binary_value.h" #include "callback.h" #include "cancelable_task_runner.h" #include "code_evaluator.h" @@ -18,6 +17,7 @@ #include "isolate_memory_monitor.h" #include "js_callback_maker.h" #include "object_manipulator.h" +#include "value.h" namespace MiniRacer { @@ -25,15 +25,17 @@ Context::Context(v8::Platform* platform, RawCallback callback) : isolate_manager_(platform), isolate_object_collector_(&isolate_manager_), isolate_memory_monitor_(&isolate_manager_), - bv_factory_(isolate_manager_.Get(), &isolate_object_collector_), - callback_([this, callback](uint64_t callback_id, BinaryValue::Ptr val) { - callback(callback_id, bv_registry_.Remember(std::move(val))); + 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_, &bv_factory_, callback_), - code_evaluator_(&context_holder_, &bv_factory_, &isolate_memory_monitor_), - heap_reporter_(&bv_factory_), - object_manipulator_(&context_holder_, &bv_factory_), + js_callback_maker_(&context_holder_, &val_factory_, callback_), + code_evaluator_(&context_holder_, + &val_factory_, + &isolate_memory_monitor_), + heap_reporter_(&val_factory_), + object_manipulator_(&context_holder_, &val_factory_), cancelable_task_manager_(&isolate_manager_) {} Context::~Context() { @@ -42,8 +44,8 @@ Context::~Context() { isolate_manager_.StopJavaScript(); } -auto Context::MakeJSCallback(uint64_t callback_id) -> BinaryValueHandle* { - return bv_registry_.Remember( +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); @@ -59,54 +61,53 @@ auto Context::RunTask(Runnable runnable, uint64_t callback_id) -> uint64_t { /*runnable=*/ std::move(runnable), /*on_completed=*/ - [this, callback_id](const BinaryValue::Ptr& val) { + [this, callback_id](const Value::Ptr& val) { callback_(callback_id, val); }, /*on_canceled=*/ - [this, callback_id](const BinaryValue::Ptr& /*val*/) { + [this, callback_id](const Value::Ptr& /*val*/) { auto err = - bv_factory_.New("execution terminated", type_terminated_exception); + val_factory_.New("execution terminated", type_terminated_exception); callback_(callback_id, err); }); } -auto Context::MakeHandleConverter(BinaryValueHandle* handle, +auto Context::MakeHandleConverter(ValueHandle* handle, const char* err_msg) -> ValueHandleConverter { - return {&bv_factory_, &bv_registry_, handle, err_msg}; + return {&val_factory_, &val_registry_, handle, err_msg}; } -ValueHandleConverter::ValueHandleConverter(BinaryValueFactory* bv_factory, - BinaryValueRegistry* bv_registry, - BinaryValueHandle* handle, +ValueHandleConverter::ValueHandleConverter(ValueFactory* val_factory, + ValueRegistry* val_registry, + ValueHandle* handle, const char* err_msg) - : bv_registry_(bv_registry), - ptr_(bv_registry->FromHandle(handle)), - err_([&bv_factory, err_msg, this]() { + : val_registry_(val_registry), + ptr_(val_registry->FromHandle(handle)), + err_([&val_factory, err_msg, this]() { if (ptr_) { - return BinaryValue::Ptr(); + return Value::Ptr(); } - return bv_factory->New(err_msg, type_value_exception); + return val_factory->New(err_msg, type_value_exception); }()) {} ValueHandleConverter::operator bool() const { return static_cast(ptr_); } -auto ValueHandleConverter::GetErrorPtr() -> BinaryValue::Ptr { +auto ValueHandleConverter::GetErrorPtr() -> Value::Ptr { return err_; } -auto ValueHandleConverter::GetErrorHandle() -> BinaryValueHandle* { - return bv_registry_->Remember(err_); +auto ValueHandleConverter::GetErrorHandle() -> ValueHandle* { + return val_registry_->Remember(err_); } -auto ValueHandleConverter::GetPtr() -> BinaryValue::Ptr { +auto ValueHandleConverter::GetPtr() -> Value::Ptr { return ptr_; } -auto Context::Eval(BinaryValueHandle* code_handle, - uint64_t callback_id) -> uint64_t { +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( @@ -125,31 +126,30 @@ void Context::CancelTask(uint64_t task_id) { cancelable_task_manager_.Cancel(task_id); } -auto Context::HeapSnapshot() -> BinaryValueHandle* { - return bv_registry_.Remember(isolate_manager_ - .Run([this](v8::Isolate* isolate) mutable { - return heap_reporter_.HeapSnapshot( - isolate); - }) - .get()); +auto Context::HeapSnapshot() -> ValueHandle* { + return val_registry_.Remember(isolate_manager_ + .Run([this](v8::Isolate* isolate) mutable { + return heap_reporter_.HeapSnapshot( + isolate); + }) + .get()); } -auto Context::HeapStats() -> BinaryValueHandle* { - return bv_registry_.Remember(isolate_manager_ - .Run([this](v8::Isolate* isolate) mutable { - return heap_reporter_.HeapStats(isolate); - }) - .get()); +auto Context::HeapStats() -> ValueHandle* { + return val_registry_.Remember(isolate_manager_ + .Run([this](v8::Isolate* isolate) mutable { + return heap_reporter_.HeapStats(isolate); + }) + .get()); } -auto Context::GetIdentityHash(BinaryValueHandle* obj_handle) - -> BinaryValueHandle* { +auto Context::GetIdentityHash(ValueHandle* obj_handle) -> ValueHandle* { auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); if (!obj_hc) { return obj_hc.GetErrorHandle(); } - return bv_registry_.Remember( + return val_registry_.Remember( isolate_manager_ .Run([this, obj_ptr = obj_hc.GetPtr()](v8::Isolate* isolate) { return object_manipulator_.GetIdentityHash(isolate, obj_ptr.get()); @@ -157,14 +157,13 @@ auto Context::GetIdentityHash(BinaryValueHandle* obj_handle) .get()); } -auto Context::GetOwnPropertyNames(BinaryValueHandle* obj_handle) - -> BinaryValueHandle* { +auto Context::GetOwnPropertyNames(ValueHandle* obj_handle) -> ValueHandle* { auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); if (!obj_hc) { return obj_hc.GetErrorHandle(); } - return bv_registry_.Remember( + return val_registry_.Remember( isolate_manager_ .Run([this, obj_ptr = obj_hc.GetPtr()](v8::Isolate* isolate) { return object_manipulator_.GetOwnPropertyNames(isolate, @@ -173,9 +172,8 @@ auto Context::GetOwnPropertyNames(BinaryValueHandle* obj_handle) .get()); } -auto Context::GetObjectItem(BinaryValueHandle* obj_handle, - BinaryValueHandle* key_handle) - -> BinaryValueHandle* { +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(); @@ -186,7 +184,7 @@ auto Context::GetObjectItem(BinaryValueHandle* obj_handle, return key_hc.GetErrorHandle(); } - return bv_registry_.Remember( + return val_registry_.Remember( isolate_manager_ .Run([this, obj_ptr = obj_hc.GetPtr(), key_ptr = key_hc.GetPtr()](v8::Isolate* isolate) mutable { @@ -196,10 +194,9 @@ auto Context::GetObjectItem(BinaryValueHandle* obj_handle, .get()); } -auto Context::SetObjectItem(BinaryValueHandle* obj_handle, - BinaryValueHandle* key_handle, - BinaryValueHandle* val_handle) - -> BinaryValueHandle* { +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(); @@ -215,7 +212,7 @@ auto Context::SetObjectItem(BinaryValueHandle* obj_handle, return val_hc.GetErrorHandle(); } - return bv_registry_.Remember( + 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 { @@ -225,9 +222,8 @@ auto Context::SetObjectItem(BinaryValueHandle* obj_handle, .get()); } -auto Context::DelObjectItem(BinaryValueHandle* obj_handle, - BinaryValueHandle* key_handle) - -> BinaryValueHandle* { +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(); @@ -238,7 +234,7 @@ auto Context::DelObjectItem(BinaryValueHandle* obj_handle, return key_hc.GetErrorHandle(); } - return bv_registry_.Remember( + return val_registry_.Remember( isolate_manager_ .Run([this, obj_ptr = obj_hc.GetPtr(), key_ptr = key_hc.GetPtr()](v8::Isolate* isolate) mutable { @@ -248,17 +244,16 @@ auto Context::DelObjectItem(BinaryValueHandle* obj_handle, .get()); } -auto Context::SpliceArray(BinaryValueHandle* obj_handle, +auto Context::SpliceArray(ValueHandle* obj_handle, int32_t start, int32_t delete_count, - BinaryValueHandle* new_val_handle) - -> BinaryValueHandle* { + ValueHandle* new_val_handle) -> ValueHandle* { auto obj_hc = MakeHandleConverter(obj_handle, "Bad handle: obj"); if (!obj_hc) { return obj_hc.GetErrorHandle(); } - BinaryValue::Ptr new_val_ptr; + Value::Ptr new_val_ptr; if (new_val_handle != nullptr) { auto new_val_hc = MakeHandleConverter(new_val_handle, "Bad handle: new_val"); @@ -268,7 +263,7 @@ auto Context::SpliceArray(BinaryValueHandle* obj_handle, new_val_ptr = new_val_hc.GetPtr(); } - return bv_registry_.Remember( + return val_registry_.Remember( isolate_manager_ .Run([this, obj_ptr = obj_hc.GetPtr(), start, delete_count, new_val_ptr](v8::Isolate* isolate) { @@ -278,9 +273,8 @@ auto Context::SpliceArray(BinaryValueHandle* obj_handle, .get()); } -auto Context::ArrayPush(BinaryValueHandle* obj_handle, - BinaryValueHandle* new_val_handle) - -> BinaryValueHandle* { +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(); @@ -291,7 +285,7 @@ auto Context::ArrayPush(BinaryValueHandle* obj_handle, return new_val_hc.GetErrorHandle(); } - return bv_registry_.Remember( + return val_registry_.Remember( isolate_manager_ .Run([this, obj_ptr = obj_hc.GetPtr(), new_val_ptr = new_val_hc.GetPtr()](v8::Isolate* isolate) { @@ -301,13 +295,13 @@ auto Context::ArrayPush(BinaryValueHandle* obj_handle, .get()); } -void Context::FreeBinaryValue(BinaryValueHandle* val) { - bv_registry_.Forget(val); +void Context::FreeValue(ValueHandle* val) { + val_registry_.Forget(val); } -auto Context::CallFunction(BinaryValueHandle* func_handle, - BinaryValueHandle* this_handle, - BinaryValueHandle* argv_handle, +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) { @@ -339,8 +333,8 @@ auto Context::CallFunction(BinaryValueHandle* func_handle, callback_id); } -auto Context::BinaryValueCount() -> size_t { - return bv_registry_.Count(); +auto Context::ValueCount() -> size_t { + return val_registry_.Count(); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/context.h b/src/v8_py_frontend/context.h index 347f19b3..deb9fd4a 100644 --- a/src/v8_py_frontend/context.h +++ b/src/v8_py_frontend/context.h @@ -4,7 +4,6 @@ #include #include #include -#include "binary_value.h" #include "callback.h" #include "cancelable_task_runner.h" #include "code_evaluator.h" @@ -15,6 +14,7 @@ #include "isolate_object_collector.h" #include "js_callback_maker.h" #include "object_manipulator.h" +#include "value.h" namespace MiniRacer { @@ -37,50 +37,49 @@ class Context { [[nodiscard]] auto IsHardMemoryLimitReached() const -> bool; void ApplyLowMemoryNotification(); - void FreeBinaryValue(BinaryValueHandle* val); + void FreeValue(ValueHandle* val); template - auto AllocBinaryValue(Params&&... params) -> BinaryValueHandle*; + auto AllocValue(Params&&... params) -> ValueHandle*; void CancelTask(uint64_t task_id); - auto HeapSnapshot() -> BinaryValueHandle*; - auto HeapStats() -> BinaryValueHandle*; - auto Eval(BinaryValueHandle* code_handle, + auto HeapSnapshot() -> ValueHandle*; + auto HeapStats() -> ValueHandle*; + auto Eval(ValueHandle* code_handle, uint64_t callback_id) -> uint64_t; - auto MakeJSCallback(uint64_t callback_id) -> BinaryValueHandle*; - auto GetIdentityHash(BinaryValueHandle* obj_handle) -> BinaryValueHandle*; - auto GetOwnPropertyNames(BinaryValueHandle* obj_handle) -> BinaryValueHandle*; - auto GetObjectItem(BinaryValueHandle* obj_handle, - BinaryValueHandle* key_handle) -> BinaryValueHandle*; - auto SetObjectItem(BinaryValueHandle* obj_handle, - BinaryValueHandle* key_handle, - BinaryValueHandle* val_handle) -> BinaryValueHandle*; - auto DelObjectItem(BinaryValueHandle* obj_handle, - BinaryValueHandle* key_handle) -> BinaryValueHandle*; - auto SpliceArray(BinaryValueHandle* obj_handle, + auto MakeJSCallback(uint64_t callback_id) -> ValueHandle*; + auto GetIdentityHash(ValueHandle* obj_handle) -> ValueHandle*; + auto GetOwnPropertyNames(ValueHandle* obj_handle) -> ValueHandle*; + auto GetObjectItem(ValueHandle* obj_handle, + ValueHandle* key_handle) -> ValueHandle*; + auto SetObjectItem(ValueHandle* obj_handle, + ValueHandle* key_handle, + ValueHandle* val_handle) -> ValueHandle*; + auto DelObjectItem(ValueHandle* obj_handle, + ValueHandle* key_handle) -> ValueHandle*; + auto SpliceArray(ValueHandle* obj_handle, int32_t start, int32_t delete_count, - BinaryValueHandle* new_val_handle) -> BinaryValueHandle*; - auto ArrayPush(BinaryValueHandle* obj_handle, - BinaryValueHandle* new_val_handle) -> BinaryValueHandle*; - auto CallFunction(BinaryValueHandle* func_handle, - BinaryValueHandle* this_handle, - BinaryValueHandle* argv_handle, - + ValueHandle* new_val_handle) -> ValueHandle*; + auto ArrayPush(ValueHandle* obj_handle, + ValueHandle* new_val_handle) -> ValueHandle*; + auto CallFunction(ValueHandle* func_handle, + ValueHandle* this_handle, + ValueHandle* argv_handle, uint64_t callback_id) -> uint64_t; - auto BinaryValueCount() -> size_t; + auto ValueCount() -> size_t; private: template auto RunTask(Runnable runnable, uint64_t callback_id) -> uint64_t; - auto MakeHandleConverter(BinaryValueHandle* handle, + auto MakeHandleConverter(ValueHandle* handle, const char* err_msg) -> ValueHandleConverter; IsolateManager isolate_manager_; IsolateObjectCollector isolate_object_collector_; IsolateMemoryMonitor isolate_memory_monitor_; - BinaryValueFactory bv_factory_; - BinaryValueRegistry bv_registry_; + ValueFactory val_factory_; + ValueRegistry val_registry_; CallbackFn callback_; ContextHolder context_holder_; JSCallbackMaker js_callback_maker_; @@ -92,21 +91,21 @@ class Context { class ValueHandleConverter { public: - ValueHandleConverter(BinaryValueFactory* bv_factory, - BinaryValueRegistry* bv_registry, - BinaryValueHandle* handle, + ValueHandleConverter(ValueFactory* val_factory, + ValueRegistry* val_registry, + ValueHandle* handle, const char* err_msg); explicit operator bool() const; - auto GetErrorPtr() -> BinaryValue::Ptr; - auto GetErrorHandle() -> BinaryValueHandle*; - auto GetPtr() -> BinaryValue::Ptr; + auto GetErrorPtr() -> Value::Ptr; + auto GetErrorHandle() -> ValueHandle*; + auto GetPtr() -> Value::Ptr; private: - BinaryValueRegistry* bv_registry_; - BinaryValue::Ptr ptr_; - BinaryValue::Ptr err_; + ValueRegistry* val_registry_; + Value::Ptr ptr_; + Value::Ptr err_; }; inline void Context::SetHardMemoryLimit(size_t limit) { @@ -130,10 +129,9 @@ inline void Context::ApplyLowMemoryNotification() { } template -inline auto Context::AllocBinaryValue(Params&&... params) - -> BinaryValueHandle* { - return bv_registry_.Remember( - bv_factory_.New(std::forward(params)...)); +inline auto Context::AllocValue(Params&&... params) -> ValueHandle* { + return val_registry_.Remember( + val_factory_.New(std::forward(params)...)); } } // namespace MiniRacer diff --git a/src/v8_py_frontend/context_holder.cc b/src/v8_py_frontend/context_holder.cc index 3a106568..e74f137f 100644 --- a/src/v8_py_frontend/context_holder.cc +++ b/src/v8_py_frontend/context_holder.cc @@ -17,7 +17,7 @@ ContextHolder::ContextHolder(IsolateManager* isolate_manager) const v8::Isolate::Scope isolate_scope(isolate); const v8::HandleScope handle_scope(isolate); - return std::make_unique>( + return std::make_unique>( isolate, v8::Context::New(isolate)); }) .get()) {} diff --git a/src/v8_py_frontend/context_holder.h b/src/v8_py_frontend/context_holder.h index e3fc8e43..35f3754a 100644 --- a/src/v8_py_frontend/context_holder.h +++ b/src/v8_py_frontend/context_holder.h @@ -19,14 +19,14 @@ class ContextHolder { ContextHolder(ContextHolder&&) = delete; auto operator=(ContextHolder&& other) -> ContextHolder& = delete; - auto Get() -> v8::Persistent*; + auto Get() -> v8::Global*; private: IsolateManager* isolate_manager_; - std::unique_ptr> context_; + std::unique_ptr> context_; }; -inline auto ContextHolder::Get() -> v8::Persistent* { +inline auto ContextHolder::Get() -> v8::Global* { return context_.get(); } diff --git a/src/v8_py_frontend/exports.cc b/src/v8_py_frontend/exports.cc index 1d6aba73..21b79bcf 100644 --- a/src/v8_py_frontend/exports.cc +++ b/src/v8_py_frontend/exports.cc @@ -5,10 +5,10 @@ #include #include #include -#include "binary_value.h" #include "callback.h" #include "context.h" #include "context_factory.h" +#include "value.h" namespace { auto GetContext(uint64_t context_id) -> std::shared_ptr { @@ -27,7 +27,7 @@ auto GetContext(uint64_t context_id) -> std::shared_ptr { // NOLINTBEGIN(bugprone-easily-swappable-parameters) LIB_EXPORT auto mr_eval(uint64_t context_id, - MiniRacer::BinaryValueHandle* code_handle, + MiniRacer::ValueHandle* code_handle, uint64_t callback_id) -> uint64_t { auto context = GetContext(context_id); if (!context) { @@ -65,46 +65,46 @@ LIB_EXPORT auto mr_context_count() -> size_t { } LIB_EXPORT void mr_free_value(uint64_t context_id, - MiniRacer::BinaryValueHandle* val_handle) { + MiniRacer::ValueHandle* val_handle) { auto context = GetContext(context_id); if (!context) { return; } - context->FreeBinaryValue(val_handle); + context->FreeValue(val_handle); } LIB_EXPORT auto mr_alloc_int_val(uint64_t context_id, int64_t val, - MiniRacer::BinaryTypes type) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueTypes type) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; } - return context->AllocBinaryValue(val, type); + return context->AllocValue(val, type); } LIB_EXPORT auto mr_alloc_double_val(uint64_t context_id, double val, - MiniRacer::BinaryTypes type) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueTypes type) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; } - return context->AllocBinaryValue(val, type); + return context->AllocValue(val, type); } LIB_EXPORT auto mr_alloc_string_val(uint64_t context_id, char* val, uint64_t len, - MiniRacer::BinaryTypes type) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueTypes type) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; } - return context->AllocBinaryValue(std::string_view(val, len), type); + return context->AllocValue(std::string_view(val, len), type); } LIB_EXPORT void mr_cancel_task(uint64_t context_id, uint64_t task_id) { @@ -115,8 +115,7 @@ LIB_EXPORT void mr_cancel_task(uint64_t context_id, uint64_t task_id) { context->CancelTask(task_id); } -LIB_EXPORT auto mr_heap_stats(uint64_t context_id) - -> MiniRacer::BinaryValueHandle* { +LIB_EXPORT auto mr_heap_stats(uint64_t context_id) -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -165,7 +164,7 @@ LIB_EXPORT void mr_low_memory_notification(uint64_t context_id) { } LIB_EXPORT auto mr_make_js_callback(uint64_t context_id, uint64_t callback_id) - -> MiniRacer::BinaryValueHandle* { + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -182,8 +181,8 @@ LIB_EXPORT auto mr_v8_is_using_sandbox() -> bool { } LIB_EXPORT auto mr_get_identity_hash(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueHandle* obj_handle) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -191,9 +190,9 @@ LIB_EXPORT auto mr_get_identity_hash(uint64_t context_id, return context->GetIdentityHash(obj_handle); } -LIB_EXPORT auto mr_get_own_property_names( - uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle) -> MiniRacer::BinaryValueHandle* { +LIB_EXPORT auto mr_get_own_property_names(uint64_t context_id, + MiniRacer::ValueHandle* obj_handle) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -202,9 +201,9 @@ LIB_EXPORT auto mr_get_own_property_names( } LIB_EXPORT auto mr_get_object_item(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle, - MiniRacer::BinaryValueHandle* key_handle) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueHandle* obj_handle, + MiniRacer::ValueHandle* key_handle) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -213,10 +212,10 @@ LIB_EXPORT auto mr_get_object_item(uint64_t context_id, } LIB_EXPORT auto mr_set_object_item(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle, - MiniRacer::BinaryValueHandle* key_handle, - MiniRacer::BinaryValueHandle* val_handle) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueHandle* obj_handle, + MiniRacer::ValueHandle* key_handle, + MiniRacer::ValueHandle* val_handle) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -225,9 +224,9 @@ LIB_EXPORT auto mr_set_object_item(uint64_t context_id, } LIB_EXPORT auto mr_del_object_item(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle, - MiniRacer::BinaryValueHandle* key_handle) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueHandle* obj_handle, + MiniRacer::ValueHandle* key_handle) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -236,11 +235,11 @@ LIB_EXPORT auto mr_del_object_item(uint64_t context_id, } LIB_EXPORT auto mr_splice_array(uint64_t context_id, - MiniRacer::BinaryValueHandle* array_handle, + MiniRacer::ValueHandle* array_handle, int32_t start, int32_t delete_count, - MiniRacer::BinaryValueHandle* new_val_handle) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueHandle* new_val_handle) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -250,9 +249,9 @@ LIB_EXPORT auto mr_splice_array(uint64_t context_id, } LIB_EXPORT auto mr_array_push(uint64_t context_id, - MiniRacer::BinaryValueHandle* array_handle, - MiniRacer::BinaryValueHandle* new_val_handle) - -> MiniRacer::BinaryValueHandle* { + MiniRacer::ValueHandle* array_handle, + MiniRacer::ValueHandle* new_val_handle) + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -261,9 +260,9 @@ LIB_EXPORT auto mr_array_push(uint64_t context_id, } LIB_EXPORT auto mr_call_function(uint64_t context_id, - MiniRacer::BinaryValueHandle* func_handle, - MiniRacer::BinaryValueHandle* this_handle, - MiniRacer::BinaryValueHandle* argv_handle, + MiniRacer::ValueHandle* func_handle, + MiniRacer::ValueHandle* this_handle, + MiniRacer::ValueHandle* argv_handle, uint64_t callback_id) -> uint64_t { auto context = GetContext(context_id); if (!context) { @@ -274,7 +273,7 @@ LIB_EXPORT auto mr_call_function(uint64_t context_id, } LIB_EXPORT auto mr_heap_snapshot(uint64_t context_id) - -> MiniRacer::BinaryValueHandle* { + -> MiniRacer::ValueHandle* { auto context = GetContext(context_id); if (!context) { return nullptr; @@ -287,7 +286,7 @@ LIB_EXPORT auto mr_value_count(uint64_t context_id) -> size_t { if (!context) { return 0; } - return context->BinaryValueCount(); + return context->ValueCount(); } // NOLINTEND(bugprone-easily-swappable-parameters) diff --git a/src/v8_py_frontend/exports.h b/src/v8_py_frontend/exports.h index df2fd486..78ff8bd4 100644 --- a/src/v8_py_frontend/exports.h +++ b/src/v8_py_frontend/exports.h @@ -3,8 +3,8 @@ #include #include -#include "binary_value.h" #include "callback.h" +#include "value.h" #ifdef V8_OS_WIN #define LIB_EXPORT __declspec(dllexport) @@ -33,7 +33,7 @@ LIB_EXPORT auto mr_v8_is_using_sandbox() -> bool; * * A MiniRacer context is an isolated JavaScript execution envrionment. It * contains one v8::Isolate, one v8::Context, one message loop thread, and - * one pool of BinaryValueHandles and asynchronous tasks. + * one pool of ValueHandles and asynchronous tasks. * * The given callback function pointer must point to valid memory for the * the entire lifetime of this context (assuming any async tasks are started @@ -50,7 +50,7 @@ LIB_EXPORT auto mr_init_context(MiniRacer::RawCallback callback) -> uint64_t; /** Free a MiniRacer context. * * This shuts down the v8::ISolate, v8::Context, the message loop thread, and - * frees any remaining BinaryValueHandles and asynchronous task handles. + * frees any remaining ValueHandles and asynchronous task handles. **/ LIB_EXPORT void mr_free_context(uint64_t context_id); @@ -84,9 +84,9 @@ LIB_EXPORT void mr_low_memory_notification(uint64_t context_id); * and passed to the C callback. **/ LIB_EXPORT auto mr_make_js_callback(uint64_t context_id, uint64_t callback_id) - -> MiniRacer::BinaryValueHandle*; + -> MiniRacer::ValueHandle*; -/** Allocate a BinaryValueHandle containing the given int or int-like data. +/** Allocate a ValueHandle containing the given int or int-like data. * * If used as an argument in another call, this value will be rendered into * JavaScript as boolean, number, undefined, or null, depending on the specified @@ -94,20 +94,20 @@ LIB_EXPORT auto mr_make_js_callback(uint64_t context_id, uint64_t callback_id) **/ LIB_EXPORT auto mr_alloc_int_val(uint64_t context_id, int64_t val, - MiniRacer::BinaryTypes type) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueTypes type) + -> MiniRacer::ValueHandle*; -/** Allocate a BinaryValueHandle containing the given double-precision number. +/** Allocate a ValueHandle containing the given double-precision number. * * If used as an argument in another call, this value will be rendered into * JavaScript as a number or Date, depending on the specified type. **/ LIB_EXPORT auto mr_alloc_double_val(uint64_t context_id, double val, - MiniRacer::BinaryTypes type) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueTypes type) + -> MiniRacer::ValueHandle*; -/** Allocate a BinaryValueHandle pointing to a copy of the given utf-8 string. +/** 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. @@ -115,14 +115,14 @@ LIB_EXPORT auto mr_alloc_double_val(uint64_t context_id, LIB_EXPORT auto mr_alloc_string_val(uint64_t context_id, char* val, uint64_t len, - MiniRacer::BinaryTypes type) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueTypes type) + -> MiniRacer::ValueHandle*; -/** Free the value pointed to by a BinaryValueHandle. */ +/** Free the value pointed to by a ValueHandle. */ LIB_EXPORT void mr_free_value(uint64_t context_id, - MiniRacer::BinaryValueHandle* val_handle); + MiniRacer::ValueHandle* val_handle); -/** Count the number of BinaryValueHandles which have been produced by the given +/** Count the number of ValueHandles which have been produced by the given *context and not freed yet. * * This function is intended for use in debugging only. @@ -131,16 +131,16 @@ LIB_EXPORT auto mr_value_count(uint64_t context_id) -> size_t; /** Get the V8 object identity hash for the given object. **/ LIB_EXPORT auto mr_get_identity_hash(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueHandle* obj_handle) + -> MiniRacer::ValueHandle*; /** Call JavaScript `Object.getOwnPropertyNames()`. * * Returns an array of names, or an exception in case of error. **/ -LIB_EXPORT auto mr_get_own_property_names( - uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle) -> MiniRacer::BinaryValueHandle*; +LIB_EXPORT auto mr_get_own_property_names(uint64_t context_id, + MiniRacer::ValueHandle* obj_handle) + -> MiniRacer::ValueHandle*; /** Call JavaScript `obj[key]`. * @@ -150,36 +150,36 @@ LIB_EXPORT auto mr_get_own_property_names( * found. **/ LIB_EXPORT auto mr_get_object_item(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle, - MiniRacer::BinaryValueHandle* key_handle) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueHandle* obj_handle, + MiniRacer::ValueHandle* key_handle) + -> MiniRacer::ValueHandle*; /** Call JavaScript `obj[key] = val`. * - * Returns a MiniRacer::BinaryValueHandle* which is normally true except in + * Returns a MiniRacer::ValueHandle* which is normally true except in * case of deletion failure, or an exception in case of error. * * Returns an exception of type type_key_exception if the key cannot be * found. **/ LIB_EXPORT auto mr_set_object_item(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle, - MiniRacer::BinaryValueHandle* key_handle, - MiniRacer::BinaryValueHandle* val_handle) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueHandle* obj_handle, + MiniRacer::ValueHandle* key_handle, + MiniRacer::ValueHandle* val_handle) + -> MiniRacer::ValueHandle*; /** Call JavaScript `delete obj[key]`. * - * Returns a MiniRacer::BinaryValueHandle* which is normally true except in + * Returns a MiniRacer::ValueHandle* which is normally true except in * case of deletion failure, or an exception in case of error. * * Returns an exception of type type_key_exception if the key cannot be * found. **/ LIB_EXPORT auto mr_del_object_item(uint64_t context_id, - MiniRacer::BinaryValueHandle* obj_handle, - MiniRacer::BinaryValueHandle* key_handle) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueHandle* obj_handle, + MiniRacer::ValueHandle* key_handle) + -> MiniRacer::ValueHandle*; /** Call JavaScript `Array.prototype.splice(array, start, delete_count, * [new_val])`. @@ -191,11 +191,11 @@ LIB_EXPORT auto mr_del_object_item(uint64_t context_id, * containing the deleted elements, or an exception in case of failure. **/ LIB_EXPORT auto mr_splice_array(uint64_t context_id, - MiniRacer::BinaryValueHandle* array_handle, + MiniRacer::ValueHandle* array_handle, int32_t start, int32_t delete_count, - MiniRacer::BinaryValueHandle* new_val_handle) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueHandle* new_val_handle) + -> MiniRacer::ValueHandle*; /** Call JavaScript `Array.prototype.push(array, new_val)`. * @@ -203,9 +203,9 @@ LIB_EXPORT auto mr_splice_array(uint64_t context_id, * in case of failure. **/ LIB_EXPORT auto mr_array_push(uint64_t context_id, - MiniRacer::BinaryValueHandle* array_handle, - MiniRacer::BinaryValueHandle* new_val_handle) - -> MiniRacer::BinaryValueHandle*; + MiniRacer::ValueHandle* array_handle, + MiniRacer::ValueHandle* new_val_handle) + -> MiniRacer::ValueHandle*; /** Cancel the given asynchronous task. * @@ -218,7 +218,7 @@ LIB_EXPORT void mr_cancel_task(uint64_t context_id, uint64_t task_id); * Code can be evaluated by, e.g., mr_alloc_string_val. * * This call is processed asynchronously and as such accepts a callback ID. - * The callback ID and a MiniRacer::BinaryValueHandle* containing the + * The callback ID and a MiniRacer::ValueHandle* containing the * evaluation result are passed back to the callback upon completion. A task ID * is returned which can be passed back to mr_cancel_task to cancel evaluation. * @@ -228,35 +228,34 @@ LIB_EXPORT void mr_cancel_task(uint64_t context_id, uint64_t task_id); * result. */ LIB_EXPORT auto mr_eval(uint64_t context_id, - MiniRacer::BinaryValueHandle* code_handle, + MiniRacer::ValueHandle* code_handle, uint64_t callback_id) -> uint64_t; /** Call JavaScript `func.call(this, ...argv)`. * * This call is processed asynchronously and as such accepts a callback ID. - * The callback ID and a MiniRacer::BinaryValueHandle* containing the + * The callback ID and a MiniRacer::ValueHandle* containing the * evaluation result are passed back to the callback upon completion. A task ID * is returned which can be passed back to mr_cancel_task to cancel evaluation. **/ LIB_EXPORT auto mr_call_function(uint64_t context_id, - MiniRacer::BinaryValueHandle* func_handle, - MiniRacer::BinaryValueHandle* this_handle, - MiniRacer::BinaryValueHandle* argv_handle, + MiniRacer::ValueHandle* func_handle, + MiniRacer::ValueHandle* this_handle, + MiniRacer::ValueHandle* argv_handle, uint64_t callback_id) -> uint64_t; /** Get stats for the V8 heap. * * This function is intended for use in debugging only. **/ -LIB_EXPORT auto mr_heap_stats(uint64_t context_id) - -> MiniRacer::BinaryValueHandle*; +LIB_EXPORT auto mr_heap_stats(uint64_t context_id) -> MiniRacer::ValueHandle*; /** Get a snapshot of the V8 heap. * * This function is intended for use in debugging only. **/ LIB_EXPORT auto mr_heap_snapshot(uint64_t context_id) - -> MiniRacer::BinaryValueHandle*; + -> MiniRacer::ValueHandle*; // NOLINTEND(bugprone-easily-swappable-parameters) diff --git a/src/v8_py_frontend/heap_reporter.cc b/src/v8_py_frontend/heap_reporter.cc index b9a9b31a..f7d02fbb 100644 --- a/src/v8_py_frontend/heap_reporter.cc +++ b/src/v8_py_frontend/heap_reporter.cc @@ -7,14 +7,14 @@ #include #include #include -#include "binary_value.h" +#include "value.h" namespace MiniRacer { -HeapReporter::HeapReporter(BinaryValueFactory* bv_factory) - : bv_factory_(bv_factory) {} +HeapReporter::HeapReporter(ValueFactory* val_factory) + : val_factory_(val_factory) {} -auto HeapReporter::HeapStats(v8::Isolate* isolate) -> BinaryValue::Ptr { +auto HeapReporter::HeapStats(v8::Isolate* isolate) -> Value::Ptr { const v8::Isolate::Scope isolatescope(isolate); const v8::HandleScope handle_scope(isolate); @@ -59,9 +59,9 @@ auto HeapReporter::HeapStats(v8::Isolate* isolate) -> BinaryValue::Ptr { v8::Local output; if (!v8::JSON::Stringify(context, stats_obj).ToLocal(&output) || output.IsEmpty()) { - return bv_factory_->New("error stringifying heap output", type_str_utf8); + return val_factory_->New("error stringifying heap output", type_str_utf8); } - return bv_factory_->New(context, output); + return val_factory_->New(context, output); } namespace { @@ -82,13 +82,13 @@ class StringOutputStream : public v8::OutputStream { }; } // end anonymous namespace -auto HeapReporter::HeapSnapshot(v8::Isolate* isolate) -> BinaryValue::Ptr { +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(); StringOutputStream sos; snap->Serialize(&sos); - return bv_factory_->New(sos.result(), type_str_utf8); + return val_factory_->New(sos.result(), type_str_utf8); } } // end namespace MiniRacer diff --git a/src/v8_py_frontend/heap_reporter.h b/src/v8_py_frontend/heap_reporter.h index 90cdfbdc..46285a35 100644 --- a/src/v8_py_frontend/heap_reporter.h +++ b/src/v8_py_frontend/heap_reporter.h @@ -2,20 +2,20 @@ #define INCLUDE_MINI_RACER_HEAP_REPORTER_H #include -#include "binary_value.h" +#include "value.h" namespace MiniRacer { /** Report fun facts about an isolate heap */ class HeapReporter { public: - explicit HeapReporter(BinaryValueFactory* bv_factory); + explicit HeapReporter(ValueFactory* val_factory); - auto HeapSnapshot(v8::Isolate* isolate) -> BinaryValue::Ptr; - auto HeapStats(v8::Isolate* isolate) -> BinaryValue::Ptr; + auto HeapSnapshot(v8::Isolate* isolate) -> Value::Ptr; + auto HeapStats(v8::Isolate* isolate) -> Value::Ptr; private: - BinaryValueFactory* bv_factory_; + ValueFactory* val_factory_; }; } // end namespace MiniRacer diff --git a/src/v8_py_frontend/isolate_object_collector.cc b/src/v8_py_frontend/isolate_object_collector.cc index 83a884a1..8e70d2f6 100644 --- a/src/v8_py_frontend/isolate_object_collector.cc +++ b/src/v8_py_frontend/isolate_object_collector.cc @@ -24,15 +24,13 @@ void IsolateObjectCollector::EnqueueCollectionBatchLocked() { } void IsolateObjectCollector::DoCollection() { - std::vector> batch; + std::vector> batch; { const std::lock_guard lock(mutex_); batch = std::exchange(garbage_, {}); } - for (const auto& collector : batch) { - collector(); - } + batch.clear(); const std::lock_guard lock(mutex_); if (garbage_.empty()) { @@ -44,11 +42,16 @@ void IsolateObjectCollector::DoCollection() { EnqueueCollectionBatchLocked(); } -IsolateObjectDeleter::IsolateObjectDeleter() - : isolate_object_collector_(nullptr) {} +void IsolateObjectCollector::Collect(v8::Global global) { + const std::lock_guard lock(mutex_); -IsolateObjectDeleter::IsolateObjectDeleter( - IsolateObjectCollector* isolate_object_collector) - : isolate_object_collector_(isolate_object_collector) {} + 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 index 2a995f78..20967f67 100644 --- a/src/v8_py_frontend/isolate_object_collector.h +++ b/src/v8_py_frontend/isolate_object_collector.h @@ -15,12 +15,10 @@ namespace MiniRacer { * 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::Persistent handle or decrement the ref count of a - * std::shared_ptr (which may free the BackingStore) 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::Persistent::~Persistent are exempt from this rule. So this - * class delegates deletion to the Isolate message loop. + * 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: @@ -34,8 +32,7 @@ class IsolateObjectCollector { auto operator=(IsolateObjectCollector&& other) -> IsolateObjectCollector& = delete; - template - void Collect(T* obj); + void Collect(v8::Global global); private: void EnqueueCollectionBatchLocked(); @@ -43,44 +40,11 @@ class IsolateObjectCollector { IsolateManager* isolate_manager_; std::mutex mutex_; - std::vector> garbage_; + std::vector> garbage_; std::condition_variable collection_done_cv_; bool is_collecting_; }; -/** A deleter for use with std::shared_ptr and std::unique_ptr. */ -class IsolateObjectDeleter { - public: - IsolateObjectDeleter(); - explicit IsolateObjectDeleter( - IsolateObjectCollector* isolate_object_collector); - - template - void operator()(T* handle) const; - - private: - IsolateObjectCollector* isolate_object_collector_; -}; - -template -inline void IsolateObjectCollector::Collect(gsl::owner obj) { - const std::lock_guard lock(mutex_); - - garbage_.push_back([obj]() { delete obj; }); - - if (is_collecting_) { - // There is already a collection in progress. - return; - } - - EnqueueCollectionBatchLocked(); -} - -template -void IsolateObjectDeleter::operator()(gsl::owner handle) const { - isolate_object_collector_->Collect(handle); -} - } // 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 b1aff0d6..4345c88c 100644 --- a/src/v8_py_frontend/js_callback_maker.cc +++ b/src/v8_py_frontend/js_callback_maker.cc @@ -11,21 +11,21 @@ #include #include #include -#include "binary_value.h" #include "callback.h" #include "context_holder.h" #include "id_maker.h" +#include "value.h" namespace MiniRacer { -JSCallbackCaller::JSCallbackCaller(BinaryValueFactory* bv_factory, +JSCallbackCaller::JSCallbackCaller(ValueFactory* val_factory, CallbackFn callback) - : bv_factory_(bv_factory), callback_(std::move(callback)) {} + : val_factory_(val_factory), callback_(std::move(callback)) {} void JSCallbackCaller::DoCallback(v8::Local context, uint64_t callback_id, v8::Local args) { - const BinaryValue::Ptr args_ptr = bv_factory_->New(context, args); + const Value::Ptr args_ptr = val_factory_->New(context, args); callback_(callback_id, args_ptr); } @@ -41,16 +41,16 @@ auto JSCallbackMaker::GetCallbackCallers() } JSCallbackMaker::JSCallbackMaker(ContextHolder* context_holder, - BinaryValueFactory* bv_factory, + ValueFactory* val_factory, CallbackFn callback) : context_holder_(context_holder), - bv_factory_(bv_factory), + val_factory_(val_factory), callback_caller_holder_( - std::make_shared(bv_factory, std::move(callback)), + std::make_shared(val_factory, std::move(callback)), GetCallbackCallers()) {} auto JSCallbackMaker::MakeJSCallback(v8::Isolate* isolate, - uint64_t callback_id) -> BinaryValue::Ptr { + 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); @@ -80,7 +80,7 @@ auto JSCallbackMaker::MakeJSCallback(v8::Isolate* isolate, v8::Function::New(context, &JSCallbackMaker::OnCalledStatic, data) .ToLocalChecked(); - return bv_factory_->New(context, func); + return val_factory_->New(context, func); } void JSCallbackMaker::OnCalledStatic( diff --git a/src/v8_py_frontend/js_callback_maker.h b/src/v8_py_frontend/js_callback_maker.h index f86950d2..b880a1c7 100644 --- a/src/v8_py_frontend/js_callback_maker.h +++ b/src/v8_py_frontend/js_callback_maker.h @@ -9,10 +9,10 @@ #include #include #include -#include "binary_value.h" #include "callback.h" #include "context_holder.h" #include "id_maker.h" +#include "value.h" namespace MiniRacer { @@ -23,14 +23,14 @@ namespace MiniRacer { */ class JSCallbackCaller { public: - JSCallbackCaller(BinaryValueFactory* bv_factory, CallbackFn callback); + JSCallbackCaller(ValueFactory* val_factory, CallbackFn callback); void DoCallback(v8::Local context, uint64_t callback_id, v8::Local args); private: - BinaryValueFactory* bv_factory_; + ValueFactory* val_factory_; CallbackFn callback_; }; @@ -39,11 +39,10 @@ class JSCallbackCaller { class JSCallbackMaker { public: JSCallbackMaker(ContextHolder* context_holder, - BinaryValueFactory* bv_factory, + ValueFactory* val_factory, CallbackFn callback); - auto MakeJSCallback(v8::Isolate* isolate, - uint64_t callback_id) -> BinaryValue::Ptr; + auto MakeJSCallback(v8::Isolate* isolate, uint64_t callback_id) -> Value::Ptr; private: static void OnCalledStatic(const v8::FunctionCallbackInfo& info); @@ -54,7 +53,7 @@ class JSCallbackMaker { static std::once_flag callback_callers_init_flag_; ContextHolder* context_holder_; - BinaryValueFactory* bv_factory_; + 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 2c77a60b..5cfcb62e 100644 --- a/src/v8_py_frontend/object_manipulator.cc +++ b/src/v8_py_frontend/object_manipulator.cc @@ -10,119 +10,128 @@ #include #include #include -#include "binary_value.h" #include "context_holder.h" +#include "value.h" namespace MiniRacer { ObjectManipulator::ObjectManipulator(ContextHolder* context, - BinaryValueFactory* bv_factory) - : context_(context), bv_factory_(bv_factory) {} + ValueFactory* val_factory) + : context_(context), val_factory_(val_factory) {} auto ObjectManipulator::GetIdentityHash(v8::Isolate* isolate, - BinaryValue* obj_ptr) - -> BinaryValue::Ptr { + 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); - const v8::Local local_obj_val = obj_ptr->ToValue(local_context); + const v8::Local local_obj_val = + obj_ptr->ToV8Value(isolate, local_context); const v8::Local local_obj = local_obj_val.As(); - return bv_factory_->New(static_cast(local_obj->GetIdentityHash()), - type_integer); + return val_factory_->New(static_cast(local_obj->GetIdentityHash()), + type_integer); } auto ObjectManipulator::GetOwnPropertyNames(v8::Isolate* isolate, - BinaryValue* obj_ptr) const - -> BinaryValue::Ptr { + 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); - const v8::Local local_obj_val = obj_ptr->ToValue(local_context); + const v8::Local local_obj_val = + obj_ptr->ToV8Value(isolate, local_context); const v8::Local local_obj = local_obj_val.As(); const v8::Local names = local_obj->GetPropertyNames(local_context).ToLocalChecked(); - return bv_factory_->New(local_context, names); + return val_factory_->New(local_context, names); } auto ObjectManipulator::Get(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* key_ptr) -> BinaryValue::Ptr { + 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); - const v8::Local local_obj_val = obj_ptr->ToValue(local_context); + const v8::Local local_obj_val = + obj_ptr->ToV8Value(isolate, local_context); const v8::Local local_obj = local_obj_val.As(); - const v8::Local local_key = key_ptr->ToValue(local_context); + const v8::Local local_key = + key_ptr->ToV8Value(isolate, local_context); if (!local_obj->Has(local_context, local_key).ToChecked()) { - return bv_factory_->New("No such key", type_key_exception); + return val_factory_->New("No such key", type_key_exception); } const v8::Local value = local_obj->Get(local_context, local_key).ToLocalChecked(); - return bv_factory_->New(local_context, value); + return val_factory_->New(local_context, value); } auto ObjectManipulator::Set(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* key_ptr, - BinaryValue* val_ptr) -> BinaryValue::Ptr { + 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); - const v8::Local local_obj_val = obj_ptr->ToValue(local_context); + const v8::Local local_obj_val = + obj_ptr->ToV8Value(isolate, local_context); const v8::Local local_obj = local_obj_val.As(); - const v8::Local local_key = key_ptr->ToValue(local_context); - const v8::Local local_value = val_ptr->ToValue(local_context); + const v8::Local local_key = + key_ptr->ToV8Value(isolate, local_context); + const v8::Local local_value = + val_ptr->ToV8Value(isolate, local_context); local_obj->Set(local_context, local_key, local_value).ToChecked(); - return bv_factory_->New(true); + return val_factory_->New(true); } auto ObjectManipulator::Del(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* key_ptr) -> BinaryValue::Ptr { + 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); - const v8::Local local_obj_val = obj_ptr->ToValue(local_context); + const v8::Local local_obj_val = + obj_ptr->ToV8Value(isolate, local_context); const v8::Local local_obj = local_obj_val.As(); - const v8::Local local_key = key_ptr->ToValue(local_context); + const v8::Local local_key = + key_ptr->ToV8Value(isolate, local_context); if (!local_obj->Has(local_context, local_key).ToChecked()) { - return bv_factory_->New("No such key", type_key_exception); + return val_factory_->New("No such key", type_key_exception); } - return bv_factory_->New( + return val_factory_->New( local_obj->Delete(local_context, local_key).ToChecked()); } auto ObjectManipulator::Splice(v8::Isolate* isolate, - BinaryValue* obj_ptr, + Value* obj_ptr, int32_t start, int32_t delete_count, - BinaryValue* new_val_ptr) -> BinaryValue::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); - const v8::Local local_obj_val = obj_ptr->ToValue(local_context); + const v8::Local local_obj_val = + obj_ptr->ToV8Value(isolate, local_context); 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 @@ -132,13 +141,13 @@ auto ObjectManipulator::Splice(v8::Isolate* isolate, v8::Local splice_val; if (!local_obj->Get(local_context, splice_name).ToLocal(&splice_val)) { - return bv_factory_->New("no splice method on object", - type_execute_exception); + return val_factory_->New("no splice method on object", + type_execute_exception); } if (!splice_val->IsFunction()) { - return bv_factory_->New("splice member is not a function", - type_execute_exception); + return val_factory_->New("splice member is not a function", + type_execute_exception); } const v8::Local splice_func = splice_val.As(); @@ -150,28 +159,29 @@ auto ObjectManipulator::Splice(v8::Isolate* isolate, v8::Int32::New(isolate, delete_count), }; if (new_val_ptr != nullptr) { - argv.push_back(new_val_ptr->ToValue(local_context)); + argv.push_back(new_val_ptr->ToV8Value(isolate, local_context)); } v8::MaybeLocal maybe_value = splice_func->Call( local_context, local_obj, static_cast(argv.size()), argv.data()); if (maybe_value.IsEmpty()) { - return bv_factory_->New(local_context, trycatch.Message(), - trycatch.Exception(), type_execute_exception); + return val_factory_->New(local_context, trycatch.Message(), + trycatch.Exception(), type_execute_exception); } - return bv_factory_->New(local_context, maybe_value.ToLocalChecked()); + return val_factory_->New(local_context, maybe_value.ToLocalChecked()); } auto ObjectManipulator::Push(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* new_val_ptr) -> BinaryValue::Ptr { + 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); - const v8::Local local_obj_val = obj_ptr->ToValue(local_context); + const v8::Local local_obj_val = + obj_ptr->ToV8Value(isolate, local_context); 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 @@ -181,12 +191,13 @@ auto ObjectManipulator::Push(v8::Isolate* isolate, v8::Local push_val; if (!local_obj->Get(local_context, push_name).ToLocal(&push_val)) { - return bv_factory_->New("no push method on object", type_execute_exception); + return val_factory_->New("no push method on object", + type_execute_exception); } if (!push_val->IsFunction()) { - return bv_factory_->New("push member is not a function", - type_execute_exception); + return val_factory_->New("push member is not a function", + type_execute_exception); } const v8::Local push_func = push_val.As(); @@ -194,31 +205,33 @@ auto ObjectManipulator::Push(v8::Isolate* isolate, const v8::TryCatch trycatch(isolate); std::vector> argv = { - new_val_ptr->ToValue(local_context)}; + new_val_ptr->ToV8Value(isolate, local_context)}; v8::MaybeLocal maybe_value = push_func->Call( local_context, local_obj, static_cast(argv.size()), argv.data()); if (maybe_value.IsEmpty()) { - return bv_factory_->New(local_context, trycatch.Message(), - trycatch.Exception(), type_execute_exception); + return val_factory_->New(local_context, trycatch.Message(), + trycatch.Exception(), type_execute_exception); } - return bv_factory_->New(local_context, maybe_value.ToLocalChecked()); + return val_factory_->New(local_context, maybe_value.ToLocalChecked()); } auto ObjectManipulator::Call(v8::Isolate* isolate, - BinaryValue* func_ptr, - BinaryValue* this_ptr, - BinaryValue* argv_ptr) -> BinaryValue::Ptr { + 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); - const v8::Local local_func_val = func_ptr->ToValue(local_context); + const v8::Local local_func_val = + func_ptr->ToV8Value(isolate, local_context); if (!local_func_val->IsFunction()) { - return bv_factory_->New("function is not callable", type_execute_exception); + return val_factory_->New("function is not callable", + type_execute_exception); } const v8::Local local_func = local_func_val.As(); @@ -227,14 +240,14 @@ auto ObjectManipulator::Call(v8::Isolate* isolate, if (this_ptr == nullptr) { local_this = v8::Undefined(isolate); } else { - local_this = this_ptr->ToValue(local_context); + local_this = this_ptr->ToV8Value(isolate, local_context); } const v8::Local local_argv_value = - argv_ptr->ToValue(local_context); + argv_ptr->ToV8Value(isolate, local_context); if (!local_argv_value->IsArray()) { - return bv_factory_->New("argv is not an array", type_execute_exception); + return val_factory_->New("argv is not an array", type_execute_exception); } const v8::Local local_argv_array = @@ -250,11 +263,11 @@ auto ObjectManipulator::Call(v8::Isolate* isolate, v8::MaybeLocal maybe_value = local_func->Call( local_context, local_this, static_cast(argv.size()), argv.data()); if (maybe_value.IsEmpty()) { - return bv_factory_->New(local_context, trycatch.Message(), - trycatch.Exception(), type_execute_exception); + return val_factory_->New(local_context, trycatch.Message(), + trycatch.Exception(), type_execute_exception); } - return bv_factory_->New(local_context, maybe_value.ToLocalChecked()); + return val_factory_->New(local_context, 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 031fe92d..7977c0f9 100644 --- a/src/v8_py_frontend/object_manipulator.h +++ b/src/v8_py_frontend/object_manipulator.h @@ -4,8 +4,8 @@ #include #include #include -#include "binary_value.h" #include "context_holder.h" +#include "value.h" namespace MiniRacer { @@ -14,41 +14,36 @@ namespace MiniRacer { * * All methods in this function assume that the caller holds the Isolate lock * (i.e., is operating from the isolate message pump), and memory management of - * the BinaryValue pointers is done by the caller. */ + * the Value pointers is done by the caller. */ class ObjectManipulator { public: - ObjectManipulator(ContextHolder* context, BinaryValueFactory* bv_factory); + ObjectManipulator(ContextHolder* context, ValueFactory* val_factory); - auto GetIdentityHash(v8::Isolate* isolate, - BinaryValue* obj_ptr) -> BinaryValue::Ptr; + auto GetIdentityHash(v8::Isolate* isolate, Value* obj_ptr) -> Value::Ptr; auto GetOwnPropertyNames(v8::Isolate* isolate, - BinaryValue* obj_ptr) const -> BinaryValue::Ptr; - auto Get(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* key_ptr) -> BinaryValue::Ptr; + Value* obj_ptr) const -> Value::Ptr; + auto Get(v8::Isolate* isolate, Value* obj_ptr, Value* key_ptr) -> Value::Ptr; auto Set(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* key_ptr, - BinaryValue* val_ptr) -> BinaryValue::Ptr; - auto Del(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* key_ptr) -> BinaryValue::Ptr; + 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, - BinaryValue* obj_ptr, + Value* obj_ptr, int32_t start, int32_t delete_count, - BinaryValue* new_val_ptr) -> BinaryValue::Ptr; + Value* new_val_ptr) -> Value::Ptr; auto Push(v8::Isolate* isolate, - BinaryValue* obj_ptr, - BinaryValue* new_val_ptr) -> BinaryValue::Ptr; + Value* obj_ptr, + Value* new_val_ptr) -> Value::Ptr; auto Call(v8::Isolate* isolate, - BinaryValue* func_ptr, - BinaryValue* this_ptr, - BinaryValue* argv_ptr) -> BinaryValue::Ptr; + Value* func_ptr, + Value* this_ptr, + Value* argv_ptr) -> Value::Ptr; private: ContextHolder* context_; - BinaryValueFactory* bv_factory_; + ValueFactory* val_factory_; }; } // end namespace MiniRacer diff --git a/src/v8_py_frontend/binary_value.cc b/src/v8_py_frontend/value.cc similarity index 62% rename from src/v8_py_frontend/binary_value.cc rename to src/v8_py_frontend/value.cc index 473318ba..0a83fdd7 100644 --- a/src/v8_py_frontend/binary_value.cc +++ b/src/v8_py_frontend/value.cc @@ -1,4 +1,4 @@ -#include "binary_value.h" +#include "value.h" #include #include @@ -23,11 +23,11 @@ namespace MiniRacer { // NOLINTBEGIN(cppcoreguidelines-pro-type-union-access) -BinaryValue::BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - v8::Local context, - v8::Local value) - : isolate_(isolate), isolate_object_deleter_(isolate_object_deleter) { +Value::Value(v8::Isolate* isolate, + IsolateObjectCollector* isolate_object_collector, + v8::Local context, + v8::Local value) + : isolate_object_collector_(isolate_object_collector) { if (value->IsNull()) { handle_.type = type_null; } else if (value->IsUndefined()) { @@ -48,10 +48,10 @@ BinaryValue::BinaryValue(v8::Isolate* isolate, handle_.int_val = (value->IsTrue() ? 1 : 0); } else if (value->IsFunction()) { handle_.type = type_function; - SavePersistentHandle(value); + SaveGlobalHandle(isolate, value); } else if (value->IsSymbol()) { handle_.type = type_symbol; - SavePersistentHandle(value); + SaveGlobalHandle(isolate, value); } else if (value->IsDate()) { handle_.type = type_date; const v8::Local date = v8::Local::Cast(value); @@ -65,62 +65,63 @@ BinaryValue::BinaryValue(v8::Isolate* isolate, handle_.type = type_str_utf8; handle_.len = static_cast(rstr->Utf8LengthV2(isolate)); // in bytes const size_t capacity = handle_.len + 1; - msg_.resize(capacity); - rstr->WriteUtf8V2(isolate, msg_.data(), capacity); - handle_.bytes = msg_.data(); + 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(value); - SavePersistentHandle(value); + CreateBackingStoreRef(isolate, value); } else if (value->IsPromise()) { handle_.type = type_promise; - SavePersistentHandle(value); + SaveGlobalHandle(isolate, value); } else if (value->IsArray()) { handle_.type = type_array; - SavePersistentHandle(value); + SaveGlobalHandle(isolate, value); } else if (value->IsObject()) { handle_.type = type_object; - SavePersistentHandle(value); + SaveGlobalHandle(isolate, value); } } -BinaryValue::BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - std::string_view val, - BinaryTypes type) - : isolate_(isolate), isolate_object_deleter_(isolate_object_deleter) { +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; - msg_.resize(handle_.len + 1); - std::copy(val.begin(), val.end(), msg_.data()); - msg_[handle_.len] = '\0'; - handle_.bytes = msg_.data(); + 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(); } -BinaryValue::BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - bool val) - : isolate_(isolate), isolate_object_deleter_(isolate_object_deleter) { +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; } -BinaryValue::BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - int64_t val, - BinaryTypes type) - : isolate_(isolate), isolate_object_deleter_(isolate_object_deleter) { +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; } -BinaryValue::BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - double val, - BinaryTypes type) - : isolate_(isolate), isolate_object_deleter_(isolate_object_deleter) { +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; @@ -191,46 +192,54 @@ auto ExceptionToString(v8::Isolate* isolate, } } // end anonymous namespace -BinaryValue::BinaryValue(v8::Isolate* isolate, - IsolateObjectDeleter isolate_object_deleter, - v8::Local context, - v8::Local message, - v8::Local exception_obj, - BinaryTypes result_type) - : BinaryValue(isolate, - isolate_object_deleter, - ExceptionToString(isolate, context, message, exception_obj), - result_type) {} - -auto BinaryValue::ToValue(v8::Local context) - -> v8::Local { - // If we've saved a handle to a v8::Persistent, we can return the exact v8 - // value to which this BinaryValue refers: - if (persistent_handle_) { - return persistent_handle_->Get(isolate_); +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 - // BinaryValueHandle: + // ValueHandle: if (handle_.type == type_null) { - return v8::Null(isolate_); + return v8::Null(isolate); } if (handle_.type == type_undefined) { - return v8::Undefined(isolate_); + return v8::Undefined(isolate); } if (handle_.type == type_integer) { - return v8::Integer::New(isolate_, static_cast(handle_.int_val)); + return v8::Integer::New(isolate, static_cast(handle_.int_val)); } if (handle_.type == type_double) { - return v8::Number::New(isolate_, handle_.double_val); + return v8::Number::New(isolate, handle_.double_val); } if (handle_.type == type_bool) { - return v8::Boolean::New(isolate_, handle_.int_val != 0); + return v8::Boolean::New(isolate, handle_.int_val != 0); } if (handle_.type == type_date) { @@ -238,29 +247,29 @@ auto BinaryValue::ToValue(v8::Local context) } if (handle_.type == type_str_utf8) { - return v8::String::NewFromUtf8(isolate_, handle_.bytes, + return v8::String::NewFromUtf8(isolate, handle_.bytes, v8::NewStringType::kNormal, static_cast(handle_.len)) .ToLocalChecked(); } // Unknown type! - return v8::Undefined(isolate_); + return v8::Undefined(isolate); } -auto BinaryValue::GetHandle() -> BinaryValueHandle* { +auto Value::GetHandle() -> ValueHandle* { return &handle_; } -void BinaryValue::SavePersistentHandle(v8::Local value) { - persistent_handle_ = {new v8::Persistent(isolate_, value), - isolate_object_deleter_}; +void Value::SaveGlobalHandle(v8::Isolate* isolate, v8::Local value) { + data_.emplace>(isolate, value); } -void BinaryValue::CreateBackingStoreRef(v8::Local value) { - // For ArrayBuffer and friends, we store a reference to the ArrayBuffer - // shared_ptr in this BinaryValue instance, and return a pointer - // *into* the buffer to the Python side. +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; @@ -289,35 +298,29 @@ void BinaryValue::CreateBackingStoreRef(v8::Local value) { // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) handle_.len = size; - // We take the unusual step of wrapping a shared_ptr in a unique_ptr so we - // can control exactly where the underlying BackingStore is destroyed (that - // is, *in the message loop thread*). - backing_store_ = {new std::shared_ptr(backing_store), - isolate_object_deleter_}; + SaveGlobalHandle(isolate, value); } // NOLINTEND(cppcoreguidelines-pro-type-union-access) -BinaryValueFactory::BinaryValueFactory( - v8::Isolate* isolate, - IsolateObjectCollector* isolate_object_collector) +ValueFactory::ValueFactory(v8::Isolate* isolate, + IsolateObjectCollector* isolate_object_collector) : isolate_(isolate), isolate_object_collector_(isolate_object_collector) {} -auto BinaryValueRegistry::Remember(BinaryValue::Ptr ptr) -> BinaryValueHandle* { +auto ValueRegistry::Remember(Value::Ptr ptr) -> ValueHandle* { const std::lock_guard lock(mutex_); - BinaryValueHandle* handle = ptr->GetHandle(); + ValueHandle* handle = ptr->GetHandle(); values_[handle] = std::move(ptr); return handle; } -void BinaryValueRegistry::Forget(BinaryValueHandle* handle) { +void ValueRegistry::Forget(ValueHandle* handle) { const std::lock_guard lock(mutex_); values_.erase(handle); } -auto BinaryValueRegistry::FromHandle(BinaryValueHandle* handle) - -> BinaryValue::Ptr { - // Track all created binary values to relieve Python of the duty of garbage +auto ValueRegistry::FromHandle(ValueHandle* handle) -> Value::Ptr { + // 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); @@ -327,7 +330,7 @@ auto BinaryValueRegistry::FromHandle(BinaryValueHandle* handle) return iter->second; } -auto BinaryValueRegistry::Count() -> size_t { +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 new file mode 100644 index 00000000..0df4b033 --- /dev/null +++ b/src/v8_py_frontend/value.h @@ -0,0 +1,174 @@ +#ifndef INCLUDE_MINI_RACER_VALUE_H +#define INCLUDE_MINI_RACER_VALUE_H + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "isolate_object_collector.h" + +namespace MiniRacer { + +enum ValueTypes : uint8_t { + type_invalid = 0, + type_null = 1, + type_bool = 2, + type_integer = 3, + type_double = 4, + type_str_utf8 = 5, + type_array = 6, + // type_hash = 7, // deprecated + type_date = 8, + type_symbol = 9, + type_object = 10, + type_undefined = 11, + + type_function = 100, + type_shared_array_buffer = 101, + type_array_buffer = 102, + type_promise = 103, + + type_execute_exception = 200, + type_parse_exception = 201, + type_oom_exception = 202, + type_timeout_exception = 203, + type_terminated_exception = 204, + type_value_exception = 205, + type_key_exception = 206, +}; + +// NOLINTBEGIN(misc-non-private-member-variables-in-classes) +// NOLINTBEGIN(cppcoreguidelines-owning-memory) +// NOLINTBEGIN(cppcoreguidelines-pro-type-member-init) +// NOLINTBEGIN(hicpp-member-init) +/** A simplified structure designed for sharing data with non-C++ code over a C + * foreign function API (e.g., Python ctypes). This object directly provides + * values for some simple types (e.g., numbers and strings), and also acts as a + * handle for the non-C++ code to manage opaque data via our APIs. */ +struct ValueHandle { + union { + char* bytes; + int64_t int_val; + double double_val; + }; + size_t len; + ValueTypes type = type_invalid; +} __attribute__((packed)); +// NOLINTEND(hicpp-member-init) +// NOLINTEND(cppcoreguidelines-pro-type-member-init) +// NOLINTEND(cppcoreguidelines-owning-memory) +// NOLINTEND(misc-non-private-member-variables-in-classes) + +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; + Value& operator=(const Value& other) = delete; + Value& operator=(Value&& other) noexcept = delete; + + using Ptr = std::shared_ptr; + + auto ToV8Value(v8::Isolate* isolate, + v8::Local context) -> v8::Local; + + friend class ValueRegistry; + + private: + auto GetHandle() -> ValueHandle*; + 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_; +}; + +class ValueFactory { + public: + explicit ValueFactory(v8::Isolate* isolate, + IsolateObjectCollector* isolate_object_collector); + + template + auto New(Params&&... params) -> Value::Ptr; + + private: + v8::Isolate* isolate_; + IsolateObjectCollector* isolate_object_collector_; +}; + +/** We return handles to Values to the MiniRacer user side (i.e., + * Python), as raw pointers. To ensure we keep those handles alive while Python + * is using them, we register them in a map, contained within this class. + */ +class ValueRegistry { + public: + /** Record the value in an internal map, so we don't destroy it when + * returning a value handle to the MiniRacer user (i.e., the + * Python side). + */ + auto Remember(Value::Ptr ptr) -> ValueHandle*; + + /** Unrecord a value so it can be garbage collected (once any other + * shared_ptr references are dropped). + */ + void Forget(ValueHandle* handle); + + /** "Re-hydrate" a value from just its handle (only works if it was + * "Remembered") */ + auto FromHandle(ValueHandle* handle) -> Value::Ptr; + + /** Count the total number of remembered values, for test purposes. */ + auto Count() -> size_t; + + private: + std::mutex mutex_; + 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/gc_check.py b/tests/gc_check.py index ddce4cb2..7718a8d8 100644 --- a/tests/gc_check.py +++ b/tests/gc_check.py @@ -35,7 +35,7 @@ def assert_no_v8_objects(mr: MiniRacer) -> None: # Thus should only be reachable if we forgot to wrap an incoming pointer with a # ValueHandle (because ValueHandle.__del__ should otherwise take care of disposing # the C++ object): - assert ctx.value_count() == 0, "Foud uncollected BinaryValues on the C++ side" + assert ctx.value_count() == 0, "Foud uncollected Values on the C++ side" async def async_assert_no_v8_objects(mr: MiniRacer) -> None: @@ -52,4 +52,4 @@ async def async_assert_no_v8_objects(mr: MiniRacer) -> None: # Thus should only be reachable if we forgot to wrap an incoming pointer with a # ValueHandle (because ValueHandle.__del__ should otherwise take care of disposing # the C++ object): - assert ctx.value_count() == 0, "Foud uncollected BinaryValues on the C++ side" + assert ctx.value_count() == 0, "Foud uncollected Values on the C++ side" diff --git a/tests/test_heap.py b/tests/test_heap.py index 1c449d47..2d09a748 100644 --- a/tests/test_heap.py +++ b/tests/test_heap.py @@ -20,3 +20,26 @@ def test_heap_snapshot() -> None: assert mr.heap_snapshot()["strings"] assert_no_v8_objects(mr) + + +def test_no_handle_leak() -> None: + mr = MiniRacer() + + big_obj_maker = "Array.from({ length: 1000000 }, (_, i) => i)" + + mr.eval(big_obj_maker) + + one_object_size = mr.heap_stats()["used_heap_size"] + + for _ in range(100): + mr.eval(big_obj_maker) + + many_object_size = mr.heap_stats()["used_heap_size"] + + # It's okay if making a lot of big objects and disposing causes V8 to increase the + # heap, but only to a point. Let it allocate 3x the memory for a repeated operation + # that, in principle, uses a fixed amount of memory and freess it on every loop: + allowable_ratio = 3 + assert many_object_size / one_object_size < allowable_ratio + + assert_no_v8_objects(mr) diff --git a/uv.lock b/uv.lock index 695bd8bb..29bc2b62 100644 --- a/uv.lock +++ b/uv.lock @@ -491,7 +491,7 @@ wheels = [ [[package]] name = "mini-racer" -version = "0.14.0" +version = "0.14.1" source = { editable = "." } [package.dev-dependencies] @@ -505,7 +505,6 @@ dev = [ { name = "packaging" }, { name = "pytest" }, { name = "ruff" }, - { name = "rust-just" }, { name = "setuptools", extra = ["core"] }, { name = "types-setuptools" }, ] @@ -523,7 +522,6 @@ dev = [ { name = "packaging", specifier = ">=25.0" }, { name = "pytest", specifier = ">=9.0.2" }, { name = "ruff", specifier = ">=0.14.8" }, - { name = "rust-just", specifier = ">=1.43.1" }, { name = "setuptools", extras = ["core"], specifier = ">=80.9.0" }, { name = "types-setuptools", specifier = ">=80.9.0.20250822" }, ] @@ -935,28 +933,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/6d/63/8b41cea3afd7f58eb64ac9251668ee0073789a3bc9ac6f816c8c6fef986d/ruff-0.14.8-py3-none-win_arm64.whl", hash = "sha256:965a582c93c63fe715fd3e3f8aa37c4b776777203d8e1d8aa3cc0c14424a4b99", size = 13634522, upload-time = "2025-12-04T15:06:43.212Z" }, ] -[[package]] -name = "rust-just" -version = "1.43.1" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/83/60/f3c0d2c557f89d607fef543e64f7088056484cc0ea0b4e9681b217da72e8/rust_just-1.43.1.tar.gz", hash = "sha256:dc7ab2efd8259cdcd80eff1514f7e859772bfafb21842effff870dbed2107663", size = 1428118, upload-time = "2025-11-19T07:49:20.097Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/a8/24/c5b87694c7d33848f820f016c3dd5aed6150d946d60a6e606a0e5165a084/rust_just-1.43.1-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:6fe8204a2c230abd2b3b946b3246e1f9ed80ee40de16473ede5ebdf0ded9454a", size = 1678997, upload-time = "2025-11-19T07:48:42.187Z" }, - { url = "https://files.pythonhosted.org/packages/23/b2/45c6925f7851c65436d5b8a75bc378921624a550d4ce296a1723a1b3f369/rust_just-1.43.1-py3-none-macosx_11_0_arm64.whl", hash = "sha256:04f8bd5fabcdebe8bf03d93b12352ab54ae3d831d317240be5cca7272173151d", size = 1561329, upload-time = "2025-11-19T07:48:45.23Z" }, - { url = "https://files.pythonhosted.org/packages/9f/a8/11f1418448d579c2e5fb99d7f26b15120292bd5032a3f091da8757e09cb6/rust_just-1.43.1-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:92781d23ce0e5d5815a0e43af97e44ce52d0ca2817c3e0bf4824795bc8db7516", size = 1645638, upload-time = "2025-11-19T07:48:47.854Z" }, - { url = "https://files.pythonhosted.org/packages/86/fd/5b3098d3dd442f21a25b51c9a94e82d2602a62f1d3f09c2587e5f2583f3c/rust_just-1.43.1-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:cde7c8cc0b77bd7c41c24587532b7dad1a44caa396aa2766abbcf5a424a1ac5c", size = 1616029, upload-time = "2025-11-19T07:48:50.622Z" }, - { url = "https://files.pythonhosted.org/packages/8a/c9/7dfec9cc246d9c94f14bdbd477650c5dbe5b9a71a2891e0c6c70b1e97c04/rust_just-1.43.1-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:5019283f827947b5ad8cf9947553595a1810474c56a4d15e9987d33bb05ae4c3", size = 1787480, upload-time = "2025-11-19T07:48:53.118Z" }, - { url = "https://files.pythonhosted.org/packages/e2/a5/96167b70fccc7d345309edb3971a40131dd2aca130c107a849572704b742/rust_just-1.43.1-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:c703f8383f6fc535a164f2cb3122ffa6e9c6c5d4b89c355f032e15623b344205", size = 1863173, upload-time = "2025-11-19T07:48:55.715Z" }, - { url = "https://files.pythonhosted.org/packages/eb/5e/05f537cba29a595748273f30cf3dc8757f633f25dd45464a6caca458c2f2/rust_just-1.43.1-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:478c90550ac0a287ac76e05381f57b41eedbe53956b227f176a76e5074e5aceb", size = 1855852, upload-time = "2025-11-19T07:48:58.483Z" }, - { url = "https://files.pythonhosted.org/packages/33/cc/77639b3a75edd92ea48ac1626c8b45d40ba0f2f498aad319747ad4a71c46/rust_just-1.43.1-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:31e0e695d215382bbc23f06e7ee0df138712bcce238dc027543e26173a23df31", size = 1771522, upload-time = "2025-11-19T07:49:01.28Z" }, - { url = "https://files.pythonhosted.org/packages/c6/72/a3778b9b3ee34e11c3a83df1e8ce862262158a6219efe514458cf9f1342e/rust_just-1.43.1-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:0c75cd479dc624cee83db8765a453f4ff174c71d71a90c723ba076d998d9834e", size = 1659934, upload-time = "2025-11-19T07:49:03.925Z" }, - { url = "https://files.pythonhosted.org/packages/a2/1c/6fec8e79007620137d33c5dcda463713a73338b02c5cfe15cc5e9d8a47b2/rust_just-1.43.1-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:f9665c00a3bb852aed1f3966ccfe50799c1db9b17841566db7fc1f99e5bbf4cd", size = 1636577, upload-time = "2025-11-19T07:49:06.706Z" }, - { url = "https://files.pythonhosted.org/packages/1c/5b/f40f6baa92014a039e75df14512505cebbeaf9df79c7305e4e8d2d21b26f/rust_just-1.43.1-py3-none-musllinux_1_2_i686.whl", hash = "sha256:a1dbd5b1634d40ccdfe493edd2d620a6bffbe85e410ddd2acdd929789bcde1fe", size = 1781292, upload-time = "2025-11-19T07:49:09.218Z" }, - { url = "https://files.pythonhosted.org/packages/a3/ab/7374bf4300b00fac5934e34c483a08ff2fdf7e99ce1146140e2e2e64ff05/rust_just-1.43.1-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:89ba22d438cad2356f80f45c670d794537ef59631c09dfd4938bf5cc3440975b", size = 1838422, upload-time = "2025-11-19T07:49:12.048Z" }, - { url = "https://files.pythonhosted.org/packages/61/d1/176231f6f0fac1ce300977ccfa45eae018a53ee0e5155a3be0b4dc2beb2d/rust_just-1.43.1-py3-none-win32.whl", hash = "sha256:9246f3aff7b55c452ea515d5c31b244bc709d86e31ac7c5cd5e18af8d1258989", size = 1567308, upload-time = "2025-11-19T07:49:14.557Z" }, - { url = "https://files.pythonhosted.org/packages/8b/c7/6c818dcac06844608244855753a0afb367365661b40fe6b3288bc26726a6/rust_just-1.43.1-py3-none-win_amd64.whl", hash = "sha256:28f0d898d3e04846348277a4d47231c949398102c2f6f452f52ba6506c9c7572", size = 1731800, upload-time = "2025-11-19T07:49:17.344Z" }, -] - [[package]] name = "setuptools" version = "80.9.0"