Skip to content

Commit

Permalink
Merge pull request #3469 from cloudflare/dlapid/v8_13.4
Browse files Browse the repository at this point in the history
Release cppHeap ownership to v8 in 13.4
  • Loading branch information
danlapid authored Feb 5, 2025
2 parents 9783f94 + 5c72ea7 commit 34b0212
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/workerd/jsg/setup.c++
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,7 @@ static v8::Isolate* newIsolate(v8::Isolate::CreateParams&& params, v8::CppHeap*
// fully utilized. This differs from browser environments, where a user is typically doing
// only one thing at a time and thus likely has CPU cores to spare.

// V8 *claims* to take ownership of the v8::CppHeap but actually releases ownership of it
// during v8::Isolate::Dispose.
// TODO(soon): submit a bug report/patch to v8.
// V8 takes ownership of the v8::CppHeap.
params.cpp_heap = cppHeap;

if (params.array_buffer_allocator == nullptr &&
Expand All @@ -328,7 +326,11 @@ IsolateBase::IsolateBase(const V8System& system,
kj::Own<IsolateObserver> observer)
: system(system),
cppHeap(newCppHeap(const_cast<V8PlatformWrapper*>(&system.platformWrapper))),
#if (V8_MAJOR_VERSION == 13 && V8_MINOR_VERSION >= 4) || V8_MAJOR_VERSION > 13
ptr(newIsolate(kj::mv(createParams), cppHeap.release())),
#else
ptr(newIsolate(kj::mv(createParams), cppHeap.get())),
#endif
heapTracer(ptr),
observer(kj::mv(observer)) {
jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
Expand Down Expand Up @@ -395,6 +397,7 @@ IsolateBase::IsolateBase(const V8System& system,
IsolateBase::~IsolateBase() noexcept(false) {
jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
ptr->Dispose();
// TODO(cleanup): meaningless after V8 13.4 is released.
cppHeap.reset();
;
});
Expand Down
2 changes: 2 additions & 0 deletions src/workerd/jsg/setup.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ class IsolateBase {
using Item = kj::OneOf<v8::Global<v8::Data>, RefToDelete>;

const V8System& system;
// TODO(cleanup): After v8 13.4 is fully released we can inline this into `newIsolate`
// and remove this member.
std::unique_ptr<class v8::CppHeap> cppHeap;
v8::Isolate* ptr;
kj::Maybe<kj::String> uuid;
Expand Down

0 comments on commit 34b0212

Please sign in to comment.