Fix V8 API memory management and implement more APIs (#13426)

src/bun.js/bindings/v8/V8Array.cpp

@@ -17,7 +17,7 @@ Local<Array> Array::New(Isolate* isolate, Local<Value>* elements, size_t length)
         static_cast<ArrayAllocationProfile*>(nullptr),
         reinterpret_cast<JSValue*>(elements),
         (unsigned int)length);
-    return isolate->currentHandleScope()->createLocal<Array>(array);
+    return isolate->currentHandleScope()->createLocal<Array>(isolate->vm(), array);
 }
 
 }

src/bun.js/bindings/v8/V8Boolean.cpp

@@ -10,7 +10,7 @@ bool Boolean::Value() const
 
 Local<Boolean> Boolean::New(Isolate* isolate, bool value)
 {
-    return isolate->currentHandleScope()->createLocal<Boolean>(JSC::jsBoolean(value));
+    return isolate->currentHandleScope()->createLocal<Boolean>(isolate->vm(), JSC::jsBoolean(value));
 }
 
 }

src/bun.js/bindings/v8/V8Context.cpp

@@ -0,0 +1,10 @@
+#include "V8Context.h"
+
+namespace v8 {
+
+Isolate* Context::GetIsolate()
+{
+    return reinterpret_cast<Isolate*>(localToPointer());
+}
+
+}

src/bun.js/bindings/v8/V8Context.h

@@ -6,10 +6,14 @@
 
 namespace v8 {
 
+class Isolate;
+
 // Context is always a reinterpret pointer to V8::Roots, so that inlined V8 functions can find
 // values they expect to find at fixed offsets
 class Context : public Data {
 public:
+    BUN_EXPORT Isolate* GetIsolate();
+
     JSC::VM& vm() const
     {
         return globalObject()->vm();

src/bun.js/bindings/v8/V8Data.h

@@ -56,10 +56,10 @@ class Data {
             }
 
             ObjectLayout* v8_object = reinterpret_cast<ObjectLayout*>(raw_ptr);
-            if (v8_object->map.getPtr<Map>()->instance_type == InstanceType::HeapNumber) {
-                return JSC::jsDoubleNumber(*reinterpret_cast<double*>(&v8_object->ptr));
+            if (v8_object->map()->instance_type == InstanceType::HeapNumber) {
+                return JSC::jsDoubleNumber(v8_object->asDouble());
             } else {
-                return JSC::JSValue(reinterpret_cast<JSC::JSCell*>(v8_object->ptr));
+                return JSC::JSValue(v8_object->asCell());
             }
         }
     }
@@ -97,8 +97,8 @@ class Data {
             // root points to the V8 object. The first field of the V8 object is the map, and the
             // second is a pointer to some object we have stored. So we ignore the map and recover
             // the object pointer.
-            JSC::JSCell** v8_object = reinterpret_cast<JSC::JSCell**>(root.getPtr());
-            return TaggedPointer(v8_object[1]);
+            ObjectLayout* v8_object = root.getPtr<ObjectLayout>();
+            return TaggedPointer(v8_object->asRaw());
         }
     }
 };

src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp

@@ -7,9 +7,7 @@ EscapableHandleScopeBase::EscapableHandleScopeBase(Isolate* isolate)
 {
     // at this point isolate->currentHandleScope() would just be this, so instead we have to get the
     // previous one
-    auto& handle = prev->buffer->createUninitializedHandle();
-    memset(&handle, 0xaa, sizeof(handle));
-    handle.to_v8_object = TaggedPointer(0);
+    auto& handle = prev->buffer->createEmptyHandle();
     escape_slot = &handle;
 }
 
@@ -18,7 +16,7 @@ EscapableHandleScopeBase::EscapableHandleScopeBase(Isolate* isolate)
 uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value)
 {
     *escape_slot = *reinterpret_cast<Handle*>(escape_value);
-    return reinterpret_cast<uintptr_t*>(escape_slot);
+    return &escape_slot->to_v8_object.value;
 }
 
 }

src/bun.js/bindings/v8/V8External.cpp

@@ -11,7 +11,7 @@ Local<External> External::New(Isolate* isolate, void* value)
     auto& vm = globalObject->vm();
     auto structure = globalObject->NapiExternalStructure();
     Bun::NapiExternal* val = Bun::NapiExternal::create(vm, structure, value, nullptr, nullptr);
-    return isolate->currentHandleScope()->createLocal<External>(val);
+    return isolate->currentHandleScope()->createLocal<External>(vm, val);
 }
 
 void* External::Value() const

src/bun.js/bindings/v8/V8FunctionTemplate.cpp

@@ -47,23 +47,25 @@ Local<FunctionTemplate> FunctionTemplate::New(
 
     auto globalObject = isolate->globalObject();
     auto& vm = globalObject->vm();
-    JSValue jsc_data = data.IsEmpty() ? JSC::jsUndefined() : data->localToJSValue(globalObject->V8GlobalInternals());
+    auto* globalInternals = globalObject->V8GlobalInternals();
+    JSValue jsc_data = data.IsEmpty() ? JSC::jsUndefined() : data->localToJSValue(globalInternals);
 
-    Structure* structure = globalObject->V8GlobalInternals()->functionTemplateStructure(globalObject);
+    Structure* structure = globalInternals->functionTemplateStructure(globalObject);
     auto* functionTemplate = new (NotNull, JSC::allocateCell<FunctionTemplate>(vm)) FunctionTemplate(
         vm, structure, callback, jsc_data);
     functionTemplate->finishCreation(vm);
 
-    return isolate->currentHandleScope()->createLocal<FunctionTemplate>(functionTemplate);
+    return globalInternals->currentHandleScope()->createLocal<FunctionTemplate>(vm, functionTemplate);
 }
 
 MaybeLocal<Function> FunctionTemplate::GetFunction(Local<Context> context)
 {
     auto& vm = context->vm();
     auto* globalObject = context->globalObject();
-    auto* f = Function::create(vm, globalObject->V8GlobalInternals()->v8FunctionStructure(globalObject), localToObjectPointer());
+    auto* globalInternals = globalObject->V8GlobalInternals();
+    auto* f = Function::create(vm, globalInternals->v8FunctionStructure(globalObject), localToObjectPointer());
 
-    return context->currentHandleScope()->createLocal<Function>(f);
+    return globalInternals->currentHandleScope()->createLocal<Function>(vm, f);
 }
 
 Structure* FunctionTemplate::createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
@@ -83,9 +85,7 @@ void FunctionTemplate::visitChildrenImpl(JSCell* cell, Visitor& visitor)
     ASSERT_GC_OBJECT_INHERITS(fn, info());
     Base::visitChildren(fn, visitor);
 
-    if (fn->__internals.data.isCell()) {
-        JSC::JSCell::visitChildren(fn->__internals.data.asCell(), visitor);
-    }
+    visitor.append(fn->__internals.data);
 }
 
 DEFINE_VISIT_CHILDREN(FunctionTemplate);
@@ -95,19 +95,20 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb
     auto* callee = JSC::jsDynamicCast<Function*>(callFrame->jsCallee());
     auto* functionTemplate = callee->functionTemplate();
     auto* isolate = Isolate::fromGlobalObject(JSC::jsDynamicCast<Zig::GlobalObject*>(globalObject));
+    auto& vm = globalObject->vm();
 
     WTF::Vector<TaggedPointer, 8> args(callFrame->argumentCount() + 1);
 
     HandleScope hs(isolate);
-    Local<Value> thisValue = hs.createLocal<Value>(callFrame->thisValue());
+    Local<Value> thisValue = hs.createLocal<Value>(vm, callFrame->thisValue());
     args[0] = thisValue.tagged();
 
     for (size_t i = 0; i < callFrame->argumentCount(); i++) {
-        Local<Value> argValue = hs.createLocal<Value>(callFrame->argument(i));
+        Local<Value> argValue = hs.createLocal<Value>(vm, callFrame->argument(i));
         args[i + 1] = argValue.tagged();
     }
 
-    Local<Value> data = hs.createLocal<Value>(functionTemplate->__internals.data);
+    Local<Value> data = hs.createLocal<Value>(vm, functionTemplate->__internals.data.get());
 
     ImplicitArgs implicit_args = {
         .holder = nullptr,

src/bun.js/bindings/v8/V8FunctionTemplate.h

@@ -111,11 +111,11 @@ class FunctionTemplate : public JSC::InternalFunction {
     class Internals {
     private:
         FunctionCallback callback;
-        JSC::JSValue data;
+        JSC::WriteBarrier<JSC::Unknown> data;
 
-        Internals(FunctionCallback callback_, JSC::JSValue data_)
+        Internals(FunctionCallback callback_, JSC::VM& vm, FunctionTemplate* owner, JSC::JSValue data_)
             : callback(callback_)
-            , data(data_)
+            , data(vm, owner, data_)
         {
         }
 
@@ -144,7 +144,7 @@ class FunctionTemplate : public JSC::InternalFunction {
     static JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES functionCall(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame);
 
     FunctionTemplate(JSC::VM& vm, JSC::Structure* structure, FunctionCallback callback, JSC::JSValue data)
-        : __internals(callback, data)
+        : __internals(callback, vm, this, data)
         , Base(vm, structure, functionCall, JSC::callHostFunctionAsConstructor)
     {
     }

src/bun.js/bindings/v8/V8GlobalInternals.cpp

@@ -12,6 +12,7 @@
 
 using JSC::ClassInfo;
 using JSC::LazyClassStructure;
+using JSC::LazyProperty;
 using JSC::Structure;
 using JSC::VM;
 
@@ -44,6 +45,10 @@ void GlobalInternals::finishCreation(VM& vm)
     m_V8FunctionStructure.initLater([](LazyClassStructure::Initializer& init) {
         init.setStructure(Function::createStructure(init.vm, init.global));
     });
+    m_GlobalHandles.initLater([](const LazyProperty<GlobalInternals, HandleScopeBuffer>::Initializer& init) {
+        init.set(HandleScopeBuffer::create(init.vm,
+            init.owner->handleScopeBufferStructure(init.owner->globalObject)));
+    });
 }
 
 template<typename Visitor>
@@ -57,6 +62,7 @@ void GlobalInternals::visitChildrenImpl(JSCell* cell, Visitor& visitor)
     thisObject->m_HandleScopeBufferStructure.visit(visitor);
     thisObject->m_FunctionTemplateStructure.visit(visitor);
     thisObject->m_V8FunctionStructure.visit(visitor);
+    thisObject->m_GlobalHandles.visit(visitor);
 }
 
 DEFINE_VISIT_CHILDREN_WITH_MODIFIER(JS_EXPORT_PRIVATE, GlobalInternals);

src/bun.js/bindings/v8/V8GlobalInternals.h

@@ -8,6 +8,7 @@
 namespace v8 {
 
 class HandleScope;
+class HandleScopeBuffer;
 
 class GlobalInternals : public JSC::JSCell {
 public:
@@ -53,6 +54,8 @@ class GlobalInternals : public JSC::JSCell {
         return m_V8FunctionStructure.getInitializedOnMainThread(globalObject);
     }
 
+    HandleScopeBuffer* globalHandles() const { return m_GlobalHandles.getInitializedOnMainThread(this); }
+
     HandleScope* currentHandleScope() const { return m_CurrentHandleScope; }
 
     void setCurrentHandleScope(HandleScope* handleScope) { m_CurrentHandleScope = handleScope; }
@@ -79,6 +82,8 @@ class GlobalInternals : public JSC::JSCell {
     JSC::LazyClassStructure m_FunctionTemplateStructure;
     JSC::LazyClassStructure m_V8FunctionStructure;
     HandleScope* m_CurrentHandleScope;
+    JSC::LazyProperty<GlobalInternals, HandleScopeBuffer> m_GlobalHandles;
+
     Oddball undefinedValue;
     Oddball nullValue;
     Oddball trueValue;

src/bun.js/bindings/v8/V8Handle.h

@@ -4,10 +4,53 @@
 
 namespace v8 {
 
-struct ObjectLayout {
+class ObjectLayout {
+private:
     // these two fields are laid out so that V8 can find the map
-    TaggedPointer map;
-    void* ptr;
+    TaggedPointer tagged_map;
+    union {
+        JSC::WriteBarrier<JSC::JSCell> cell;
+        double number;
+        void* raw;
+    } contents;
+
+public:
+    ObjectLayout()
+        // using a smi value for map is most likely to catch bugs as almost every access will expect
+        // map to be a pointer (and even if the assertion is bypassed, it'll be a null pointer)
+        : tagged_map(0)
+        , contents({ .raw = nullptr })
+    {
+    }
+
+    ObjectLayout(const Map* map_ptr, JSC::JSCell* cell, JSC::VM& vm, const JSC::JSCell* owner)
+        : tagged_map(const_cast<Map*>(map_ptr))
+        , contents({ .cell = JSC::WriteBarrier<JSC::JSCell>(vm, owner, cell) })
+    {
+    }
+
+    ObjectLayout(double number)
+        : tagged_map(const_cast<Map*>(&Map::heap_number_map))
+        , contents({ .number = number })
+    {
+    }
+
+    ObjectLayout(void* raw)
+        : tagged_map(const_cast<Map*>(&Map::raw_ptr_map))
+        , contents({ .raw = raw })
+    {
+    }
+
+    const Map* map() const { return tagged_map.getPtr<Map>(); }
+
+    double asDouble() const { return contents.number; }
+
+    JSC::JSCell* asCell() const { return contents.cell.get(); }
+
+    void* asRaw() const { return contents.raw; }
+
+    friend class Handle;
+    friend class HandleScopeBuffer;
 };
 
 // A handle stored in a HandleScope with layout suitable for V8's inlined functions:
@@ -24,14 +67,31 @@ struct ObjectLayout {
 //   the third field to get the actual object (either a JSCell* or a void*, depending on whether map
 //   points to Map::object_map or Map::raw_ptr_map).
 struct Handle {
-    Handle(const Map* map_, void* ptr_)
+    static_assert(offsetof(ObjectLayout, tagged_map) == 0, "ObjectLayout is wrong");
+    static_assert(offsetof(ObjectLayout, contents) == 8, "ObjectLayout is wrong");
+    static_assert(sizeof(ObjectLayout) == 16, "ObjectLayout is wrong");
+
+    Handle(const Map* map, JSC::JSCell* cell, JSC::VM& vm, const JSC::JSCell* owner)
+        : to_v8_object(&this->object)
+        , object(map, cell, vm, owner)
+    {
+    }
+
+    Handle(double number)
+        : to_v8_object(&this->object)
+        , object(number)
+    {
+    }
+
+    Handle(void* raw)
         : to_v8_object(&this->object)
-        , object({ .map = const_cast<Map*>(map_), .ptr = ptr_ })
+        , object(raw)
     {
     }
 
     Handle(int32_t smi)
         : to_v8_object(smi)
+        , object()
     {
     }
 
@@ -40,10 +100,15 @@ struct Handle {
         *this = that;
     }
 
+    Handle(const ObjectLayout* that)
+        : to_v8_object(&this->object)
+    {
+        object = *that;
+    }
+
     Handle& operator=(const Handle& that)
     {
-        object.map = that.object.map;
-        object.ptr = that.object.ptr;
+        object = that.object;
         if (that.to_v8_object.type() == TaggedPointer::Type::Smi) {
             to_v8_object = that.to_v8_object;
         } else {
@@ -52,14 +117,18 @@ struct Handle {
         return *this;
     }
 
-    Handle() {}
+    Handle()
+        : to_v8_object(0)
+        , object()
+    {
+    }
 
     bool isCell() const
     {
         if (to_v8_object.type() == TaggedPointer::Type::Smi) {
             return false;
         }
-        const Map* map_ptr = object.map.getPtr<Map>();
+        const Map* map_ptr = object.map();
         // TODO(@190n) exhaustively switch on InstanceType
         if (map_ptr == &Map::object_map || map_ptr == &Map::string_map) {
             return true;

src/bun.js/bindings/v8/V8HandleScope.cpp

@@ -15,14 +15,13 @@ HandleScope::HandleScope(Isolate* isolate_)
 HandleScope::~HandleScope()
 {
     isolate->globalInternals()->setCurrentHandleScope(prev);
+    buffer = nullptr;
 }
 
 uintptr_t* HandleScope::CreateHandle(internal::Isolate* isolate, uintptr_t value)
 {
-    // TODO figure out if this is actually used directly
-    V8_UNIMPLEMENTED();
-    // return buffer->createHandle(value);
-    return nullptr;
+    auto* handleScope = reinterpret_cast<Isolate*>(isolate)->globalInternals()->currentHandleScope();
+    return &handleScope->buffer->createHandleFromExistingHandle(TaggedPointer::fromRaw(value))->value;
 }
 
 }