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;
 }
 
 }