Simplify handling of values and add JSString

ARCHITECTURE.md

@@ -431,7 +431,7 @@ For now at least, we instead use raw pointers for this case.
 We still don't fully trust Python with the lifecyce of `ValueHandle` pointers; when
 Python passes these pointers back to C++, we still check validity by looking up the
 pointer as a key into a map (which then lets the C++ side of PyMiniRacer find the _rest_
-of the `Value` object). The C++ `MiniRacer::ValueFactory` can authoritatively destruct
+of the `Value` object). The C++ `MiniRacer::ValueRegistry` can authoritatively destruct
 any dangling `Value` objects when it exits.
 
 This last especially helps with an odd scenario introduced by Python `__del__`: the

README.md

@@ -135,6 +135,14 @@ respectively:
     [None, JSUndefined]
 ```
 
+JavaScript strings are represented as `JSString`, a subclass of `str` which enables them
+to be efficiently passed back to JavaScript:
+
+```python
+    >>> from py_mini_racer import JSString
+    >>> assert isinstance(ctx.eval("'foo'"), JSString)
+```
+
 You can prevent runaway execution in synchronous code using the `timeout_sec` parameter:
 
 ```python

src/py_mini_racer/__init__.py

@@ -24,6 +24,7 @@
     JSMappedObject,
     JSObject,
     JSPromise,
+    JSString,
     JSSymbol,
     JSUndefined,
     JSUndefinedType,
@@ -45,6 +46,7 @@
     "JSParseException",
     "JSPromise",
     "JSPromiseError",
+    "JSString",
     "JSSymbol",
     "JSTimeoutException",
     "JSUndefined",

src/py_mini_racer/_context.py

@@ -8,7 +8,7 @@
 from typing import TYPE_CHECKING, Any, NewType, Protocol, cast
 
 from py_mini_racer._dll import init_mini_racer
-from py_mini_racer._exc import JSEvalException, JSPromiseError
+from py_mini_racer._exc import JSPromiseError
 from py_mini_racer._types import (
     CancelableJSFunction,
     JSArray,
@@ -99,7 +99,7 @@ class Context:
     _active_cancelable_mr_task_callbacks: dict[int, Callable[[ValueHandle], None]] = (
         field(default_factory=dict)
     )
-    _non_cancelable_mr_task_results_queue: queue.Queue[ValueHandle] = field(
+    _non_cancelable_mr_task_results_queue: queue.Queue[RawValueHandleType] = field(
         default_factory=queue.Queue
     )
 
@@ -118,21 +118,19 @@ def handle_callback_from_v8(
         # All work on the Isolate is blocked until this callback returns. That may
         # may in turn be blocking incoming calls from Python, including other threads,
         # asyncio event loops, etc. So we need to get out fast!
-        # We limit ourselves to wrapping the incoming handle (so we don't leak memory)
-        # and enqueing the incoming work for decoupled processing.
-
-        val_handle = self._wrap_raw_handle(raw_val_handle)
+        # Just shove the handle onto a queue for decoupled processing and get out.
 
         if callback_id == _UNCANCELABLE_TASK_CALLBACK_ID:
-            self._non_cancelable_mr_task_results_queue.put(val_handle)
+            self._non_cancelable_mr_task_results_queue.put(raw_val_handle)
         else:
             self.event_loop.call_soon_threadsafe(
-                self._handle_callback_from_v8_on_event_loop, callback_id, val_handle
+                self._handle_callback_from_v8_on_event_loop, callback_id, raw_val_handle
             )
 
     def _handle_callback_from_v8_on_event_loop(
-        self, callback_id: int, val_handle: ValueHandle
+        self, callback_id: int, raw_val_handle: RawValueHandleType
     ) -> None:
+        val_handle = self._wrap_raw_handle(raw_val_handle)
         try:
             callback = self._active_cancelable_mr_task_callbacks[callback_id]
         except KeyError:
@@ -508,7 +506,7 @@ def callback(val_handle: ValueHandle) -> None:
 
             try:
                 value = self._value_handle_to_python(val_handle)
-            except JSEvalException as e:
+            except Exception as e:  # noqa: BLE001
                 future.set_exception(e)
                 return
 
@@ -538,7 +536,8 @@ def _run_uncancelable_mr_task(
         assert self.are_we_running_on_the_mini_racer_event_loop()
 
         _task_id = dll_method(self._ctx, *args, _UNCANCELABLE_TASK_CALLBACK_ID)
-        val_handle = self._non_cancelable_mr_task_results_queue.get()
+        raw_val_handle = self._non_cancelable_mr_task_results_queue.get()
+        val_handle = self._wrap_raw_handle(raw_val_handle)
         return self._value_handle_to_python(val_handle)
 
     def _value_handle_to_python(

src/py_mini_racer/_objects.py

@@ -26,6 +26,7 @@
     JSMappedObject,
     JSObject,
     JSPromise,
+    JSString,
     JSSymbol,
     JSUndefined,
     JSUndefinedType,
@@ -35,6 +36,8 @@
 if TYPE_CHECKING:
     from collections.abc import Generator, Iterator, Sequence
 
+    from typing_extensions import Self
+
     from py_mini_racer._context import Context
     from py_mini_racer._dll import RawValueHandleTypeImpl
     from py_mini_racer._value_handle import ValueHandle
@@ -192,6 +195,16 @@ def __await__(self) -> Generator[Any, None, Any]:
         return self._ctx.await_promise(self).__await__()
 
 
+class JSStringImpl(JSObjectImpl, JSString):
+    def __new__(cls, ctx: Context, handle: ValueHandle, content: str) -> Self:
+        del ctx, handle
+        return super().__new__(cls, content)
+
+    def __init__(self, ctx: Context, handle: ValueHandle, content: str) -> None:
+        del content
+        JSObjectImpl.__init__(self, ctx, handle)
+
+
 class _ArrayBufferByte(ctypes.Structure):
     # Cannot use c_ubyte directly because it uses <B
     # as an internal type but we need B for memoryview.
@@ -210,7 +223,7 @@ class _MiniRacerTypes:
     bool = 2
     integer = 3
     double = 4
-    str_utf8 = 5
+    string = 5
     array = 6
     # deprecated:
     hash = 7
@@ -222,7 +235,8 @@ class _MiniRacerTypes:
     function = 100
     shared_array_buffer = 101
     array_buffer = 102
-    promise = 103
+    array_buffer_view = 103
+    promise = 104
 
     execute_exception = 200
     parse_exception = 201
@@ -298,8 +312,10 @@ def value_handle_to_python(  # noqa: C901, PLR0911, PLR0912
             return int(val.int_val)
         if typ == _MiniRacerTypes.double:
             return float(val.double_val)
-        if typ == _MiniRacerTypes.str_utf8:
-            return str(val.bytes_val[0:length].decode("utf-8"))
+        if typ == _MiniRacerTypes.string:
+            return JSStringImpl(
+                ctx, val_handle, val.bytes_val[0:length].decode("utf-8")
+            )
         if typ == _MiniRacerTypes.function:
             return JSFunctionImpl(ctx, val_handle)
         if typ == _MiniRacerTypes.date:
@@ -308,7 +324,11 @@ def value_handle_to_python(  # noqa: C901, PLR0911, PLR0912
             return datetime.fromtimestamp(timestamp / 1000.0, timezone.utc)
         if typ == _MiniRacerTypes.symbol:
             return JSSymbolImpl(ctx, val_handle)
-        if typ in (_MiniRacerTypes.shared_array_buffer, _MiniRacerTypes.array_buffer):
+        if typ in (
+            _MiniRacerTypes.shared_array_buffer,
+            _MiniRacerTypes.array_buffer,
+            _MiniRacerTypes.array_buffer_view,
+        ):
             buf = _ArrayBufferByte * length
             cdata = buf.from_address(val.value_ptr)
             # Save a reference to the context to prevent garbage collection of the
@@ -328,7 +348,8 @@ def value_handle_to_python(  # noqa: C901, PLR0911, PLR0912
         if typ == _MiniRacerTypes.object:
             return JSMappedObjectImpl(ctx, val_handle)
 
-        raise JSConversionException
+        msg = f"Unrecognized type {typ}"
+        raise JSConversionException(msg)
 
     def python_to_value_handle(  # noqa: PLR0911
         self, ctx: Context, obj: PythonJSConvertedTypes
@@ -360,7 +381,7 @@ def python_to_value_handle(  # noqa: PLR0911
         if isinstance(obj, float):
             return ctx.create_doublish_val(obj, _MiniRacerTypes.double)
         if isinstance(obj, str):
-            return ctx.create_string_val(obj, _MiniRacerTypes.str_utf8)
+            return ctx.create_string_val(obj, _MiniRacerTypes.string)
         if isinstance(obj, datetime):
             # JS timestamps are milliseconds. In Python we are in seconds:
             return ctx.create_doublish_val(

src/py_mini_racer/_types.py

@@ -33,6 +33,10 @@ class JSObject:
     """A JavaScript object."""
 
 
+class JSString(str, JSObject):
+    __slots__ = ()
+
+
 class JSMappedObject(
     MutableMapping["PythonJSConvertedTypes", "PythonJSConvertedTypes"], JSObject
 ):

src/v8_py_frontend/BUILD.gn

@@ -14,20 +14,14 @@ v8_shared_library("mini_racer") {
     "context.cc",
     "context_factory.h",
     "context_factory.cc",
-    "context_holder.h",
-    "context_holder.cc",
     "gsl_stub.h",
     "heap_reporter.h",
     "heap_reporter.cc",
     "id_maker.h",
-    "isolate_holder.h",
-    "isolate_holder.cc",
     "isolate_manager.h",
     "isolate_manager.cc",
     "isolate_memory_monitor.h",
     "isolate_memory_monitor.cc",
-    "isolate_object_collector.h",
-    "isolate_object_collector.cc",
     "object_manipulator.h",
     "object_manipulator.cc",
     "js_callback_maker.h",

src/v8_py_frontend/callback.h

@@ -2,15 +2,12 @@
 #define INCLUDE_MINI_RACER_CALLBACK_H
 
 #include <cstdint>
-#include <functional>
 #include "value.h"
 
 namespace MiniRacer {
 
 using RawCallback = void (*)(uint64_t, ValueHandle*);
 
-using CallbackFn = std::function<void(uint64_t, Value::Ptr)>;
-
 }  // end namespace MiniRacer
 
 #endif  // INCLUDE_MINI_RACER_CALLBACK_H

src/v8_py_frontend/cancelable_task_runner.h

@@ -8,7 +8,6 @@
 #include <utility>
 #include "id_maker.h"
 #include "isolate_manager.h"
-#include "v8-isolate.h"
 
 namespace MiniRacer {
 
@@ -78,7 +77,7 @@ class CancelableTask : public CancelableTaskBase {
                           OnCompleted on_completed,
                           OnCanceled on_canceled);
 
-  void Run(v8::Isolate* isolate);
+  void Run();
   void Cancel(IsolateManager* isolate_manager) override;
 
  private:
@@ -108,10 +107,8 @@ inline auto CancelableTaskManager::Schedule(Runnable runnable,
 
   const uint64_t task_id = task_id_holder.GetId();
 
-  std::future<void> fut = isolate_manager_->Run(
-      [holder = std::move(task_id_holder), task](v8::Isolate* isolate) mutable {
-        task->Run(isolate);
-      });
+  std::future<void> fut = isolate_manager_->Schedule(
+      [holder = std::move(task_id_holder), task]() mutable { task->Run(); });
 
   task->SetFuture(std::move(fut));
 
@@ -129,8 +126,7 @@ inline CancelableTask<Runnable, OnCompleted, OnCanceled>::CancelableTask(
       state_(State::kNotStarted) {}
 
 template <typename Runnable, typename OnCompleted, typename OnCanceled>
-inline void CancelableTask<Runnable, OnCompleted, OnCanceled>::Run(
-    v8::Isolate* isolate) {
+inline void CancelableTask<Runnable, OnCompleted, OnCanceled>::Run() {
   bool was_canceled_before_run = false;
   {
     const std::lock_guard<std::mutex> lock(mutex_);
@@ -146,7 +142,7 @@ inline void CancelableTask<Runnable, OnCompleted, OnCanceled>::Run(
     return;
   }
 
-  auto result = runnable_(isolate);
+  auto result = runnable_();
 
   bool was_canceled_during_run = false;
   {

src/v8_py_frontend/code_evaluator.cc

@@ -9,32 +9,29 @@
 #include <v8-primitive.h>
 #include <v8-script.h>
 #include <v8-value.h>
-#include "context_holder.h"
+#include "isolate_manager.h"
 #include "isolate_memory_monitor.h"
 #include "value.h"
 
 namespace MiniRacer {
 
-CodeEvaluator::CodeEvaluator(ContextHolder* context,
+CodeEvaluator::CodeEvaluator(IsolateManager* isolate_manager,
                              ValueFactory* val_factory,
                              IsolateMemoryMonitor* memory_monitor)
-    : context_(context),
+    : isolate_manager_(isolate_manager),
       val_factory_(val_factory),
       memory_monitor_(memory_monitor) {}
 
-auto CodeEvaluator::Eval(v8::Isolate* isolate, Value* code_ptr) -> Value::Ptr {
-  const v8::Isolate::Scope isolate_scope(isolate);
-  const v8::HandleScope handle_scope(isolate);
-  const v8::Local<v8::Context> context = context_->Get()->Get(isolate);
-  const v8::Context::Scope context_scope(context);
+auto CodeEvaluator::Eval(Value* code_ptr) -> Value::Ptr {
+  v8::Isolate* isolate = isolate_manager_->GetIsolate();
 
   const v8::TryCatch trycatch(isolate);
 
-  const v8::Local<v8::Value> local_code_val =
-      code_ptr->ToV8Value(isolate, context);
+  const v8::Local<v8::Value> local_code_val = code_ptr->Global()->Get(isolate);
 
   if (!local_code_val->IsString()) {
-    return val_factory_->New("code is not a string", type_execute_exception);
+    return val_factory_->NewFromString("code is not a string",
+                                       type_execute_exception);
   }
 
   const v8::Local<v8::String> local_code_str = local_code_val.As<v8::String>();
@@ -43,31 +40,33 @@ auto CodeEvaluator::Eval(v8::Isolate* isolate, Value* code_ptr) -> Value::Ptr {
   v8::ScriptOrigin script_origin(
       v8::String::NewFromUtf8Literal(isolate, "<anonymous>"));
 
+  const v8::Local<v8::Context> context = isolate_manager_->GetLocalContext();
+
   v8::Local<v8::Script> script;
   if (!v8::Script::Compile(context, local_code_str, &script_origin)
            .ToLocal(&script) ||
       script.IsEmpty()) {
-    return val_factory_->New(context, trycatch.Message(), trycatch.Exception(),
-                             type_parse_exception);
+    return val_factory_->NewFromException(
+        trycatch.Message(), trycatch.Exception(), type_parse_exception);
   }
 
   v8::MaybeLocal<v8::Value> maybe_value = script->Run(context);
   if (!maybe_value.IsEmpty()) {
-    return val_factory_->New(context, maybe_value.ToLocalChecked());
+    return val_factory_->NewFromAny(maybe_value.ToLocalChecked());
   }
 
   // Didn't execute. Find an error:
   if (memory_monitor_->IsHardMemoryLimitReached()) {
-    return val_factory_->New("", type_oom_exception);
+    return val_factory_->NewFromString("", type_oom_exception);
   }
 
   ValueTypes result_type = type_execute_exception;
   if (trycatch.HasTerminated()) {
     result_type = type_terminated_exception;
   }
 
-  return val_factory_->New(context, trycatch.Message(), trycatch.Exception(),
-                           result_type);
+  return val_factory_->NewFromException(trycatch.Message(),
+                                        trycatch.Exception(), result_type);
 }
 
 }  // end namespace MiniRacer