Skip to content

Pass a jsg::Lock to wrap and unwrap so we can get the isolate from it #4246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Pass a jsg::Lock to wrap and unwrap so we can get the isolate from it #4246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
586fb62
Pass a jsg::Lock to wrap and unwrap so we can get the isolate from it
erikcorry Jun 3, 2025
7ac507e
Add the lock argument to unwrap.
erikcorry Jun 4, 2025
f182153
wip
danlapid Jun 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/workerd/io/promise-wrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,23 @@ class PromiseWrapper {
// For some reason, this wasn't needed for jsg::V8Ref<T>...

template <typename T>
v8::Local<v8::Promise> wrap(v8::Local<v8::Context> context,
v8::Local<v8::Promise> wrap(jsg::Lock& js,
v8::Local<v8::Context> context,
kj::Maybe<v8::Local<v8::Object>> creator,
kj::Promise<T> promise) {
auto& js = jsg::Lock::from(context->GetIsolate());
auto jsPromise = IoContext::current().awaitIoLegacy(js, kj::mv(promise));
return static_cast<Self&>(*this).wrap(context, kj::mv(creator), kj::mv(jsPromise));
return static_cast<Self&>(*this).wrap(js, context, kj::mv(creator), kj::mv(jsPromise));
}

template <typename T>
kj::Maybe<kj::Promise<T>> tryUnwrap(v8::Local<v8::Context> context,
kj::Maybe<kj::Promise<T>> tryUnwrap(jsg::Lock& js,
v8::Local<v8::Context> context,
v8::Local<v8::Value> handle,
kj::Promise<T>*,
kj::Maybe<v8::Local<v8::Object>> parentObject) {
auto& wrapper = static_cast<Self&>(*this);
auto jsPromise = KJ_UNWRAP_OR_RETURN(
wrapper.tryUnwrap(context, handle, (jsg::Promise<T>*)nullptr, parentObject), kj::none);
auto& js = jsg::Lock::from(context->GetIsolate());
wrapper.tryUnwrap(js, context, handle, (jsg::Promise<T>*)nullptr, parentObject), kj::none);
return IoContext::current().awaitJs(js, kj::mv(jsPromise));
}

Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/worker.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,8 +1079,8 @@ Worker::Isolate::Isolate(kj::Own<Api> apiParam,
lock->v8Isolate->SetPromiseRejectCallback([](v8::PromiseRejectMessage message) {
// TODO(cleanup): IoContext doesn't really need to be involved here. We are trying to call
// a method of ServiceWorkerGlobalScope, which is the context object. So we should be able to
// do something like unwrap(isolate->GetCurrentContext()).emitPromiseRejection(). However, JSG
// doesn't currently provide an easy way to do this.
// do something like unwrap(lock, isolate->GetCurrentContext()).emitPromiseRejection().
// However, JSG doesn't currently provide an easy way to do this.
if (IoContext::hasCurrent()) {
try {
IoContext::current().getCurrentLock().reportPromiseRejectEvent(message);
Expand Down
10 changes: 6 additions & 4 deletions src/workerd/jsg/buffersource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,20 +468,22 @@ class BufferSourceWrapper {
return "BufferSource";
}

v8::Local<v8::Value> wrap(v8::Local<v8::Context> context,
v8::Local<v8::Value> wrap(Lock& js,
v8::Local<v8::Context> context,
kj::Maybe<v8::Local<v8::Object>> creator,
BufferSource bufferSource) {
return bufferSource.getHandle(Lock::from(context->GetIsolate()));
return bufferSource.getHandle(js);
}

kj::Maybe<BufferSource> tryUnwrap(v8::Local<v8::Context> context,
kj::Maybe<BufferSource> tryUnwrap(Lock& js,
v8::Local<v8::Context> context,
v8::Local<v8::Value> handle,
BufferSource*,
kj::Maybe<v8::Local<v8::Object>> parentObject) {
if (!handle->IsArrayBuffer() && !handle->IsArrayBufferView()) {
return kj::none;
}
return BufferSource(Lock::from(context->GetIsolate()), handle);
return BufferSource(js, handle);
}
};

Expand Down
56 changes: 32 additions & 24 deletions src/workerd/jsg/function.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,20 @@ struct FunctorCallback<TypeWrapper, Ret(Args...), kj::_::Indexes<indexes...>> {
liftKj(args, [&]() {
auto isolate = args.GetIsolate();
auto context = isolate->GetCurrentContext();
auto& js = Lock::from(isolate);
auto& wrapper = TypeWrapper::from(isolate);
auto& func = extractInternalPointer<WrappableFunction<Ret(Args...)>, false>(
context, args.Data().As<v8::Object>());

if constexpr (isVoid<Ret>()) {
func(Lock::from(isolate),
wrapper.template unwrap<Args>(
context, args, indexes, TypeErrorContext::callbackArgument(indexes))...);
js, context, args, indexes, TypeErrorContext::callbackArgument(indexes))...);
} else {
return wrapper.wrap(context, args.This(),
return wrapper.wrap(js, context, args.This(),
func(Lock::from(isolate),
wrapper.template unwrap<Args>(
context, args, indexes, TypeErrorContext::callbackArgument(indexes))...));
js, context, args, indexes, TypeErrorContext::callbackArgument(indexes))...));
}
});
}
Expand All @@ -119,19 +120,20 @@ struct FunctorCallback<TypeWrapper,
auto isolate = args.GetIsolate();
auto context = isolate->GetCurrentContext();
auto& wrapper = TypeWrapper::from(isolate);
auto& js = Lock::from(isolate);
auto& func = extractInternalPointer<
WrappableFunction<Ret(const v8::FunctionCallbackInfo<v8::Value>&, Args...)>, false>(
context, args.Data().As<v8::Object>());

if constexpr (isVoid<Ret>()) {
func(Lock::from(isolate), args,
func(js, args,
wrapper.template unwrap<Args>(
context, args, indexes, TypeErrorContext::callbackArgument(indexes))...);
js, context, args, indexes, TypeErrorContext::callbackArgument(indexes))...);
} else {
return wrapper.wrap(context, args.This(),
func(Lock::from(isolate), args,
return wrapper.wrap(js, context, args.This(),
func(js, args,
wrapper.template unwrap<Args>(
context, args, indexes, TypeErrorContext::callbackArgument(indexes))...));
js, context, args, indexes, TypeErrorContext::callbackArgument(indexes))...));
}
});
}
Expand Down Expand Up @@ -321,16 +323,19 @@ class FunctionWrapper {

template <typename Func,
typename Signature = MethodSignature<decltype(&kj::Decay<Func>::operator())>>
v8::Local<v8::Function> wrap(
v8::Local<v8::Context> context, kj::Maybe<v8::Local<v8::Object>> creator, Func&& func) {
return wrap(context, creator, jsg::Function<Signature>(kj::mv(func)));
v8::Local<v8::Function> wrap(Lock& js,
v8::Local<v8::Context> context,
kj::Maybe<v8::Local<v8::Object>> creator,
Func&& func) {
return wrap(js, context, creator, jsg::Function<Signature>(kj::mv(func)));
}

template <typename Signature>
v8::Local<v8::Function> wrap(v8::Local<v8::Context> context,
v8::Local<v8::Function> wrap(Lock& js,
v8::Local<v8::Context> context,
kj::Maybe<v8::Local<v8::Object>> creator,
Function<Signature>&& func) {
v8::Isolate* isolate = context->GetIsolate();
v8::Isolate* isolate = js.v8Isolate;
return func.getOrCreateHandle(isolate, [&](Ref<WrappableFunction<Signature>>& ref) {
v8::Local<v8::Object> data;
KJ_IF_SOME(h, ref->tryGetHandle(isolate)) {
Expand Down Expand Up @@ -372,15 +377,16 @@ class FunctionWrapper {
}

template <typename Ret, typename... Args>
kj::Maybe<Constructor<Ret(Args...)>> tryUnwrap(v8::Local<v8::Context> context,
kj::Maybe<Constructor<Ret(Args...)>> tryUnwrap(Lock& js,
v8::Local<v8::Context> context,
v8::Local<v8::Value> handle,
Constructor<Ret(Args...)>*,
kj::Maybe<v8::Local<v8::Object>> parentObject) {
if (!handle->IsFunction()) {
return kj::none;
}

auto isolate = context->GetIsolate();
auto isolate = js.v8Isolate;

auto wrapperFn = [](Lock& js, v8::Local<v8::Value> receiver, v8::Local<v8::Function> func,
Args... args) -> Ret {
Expand All @@ -390,11 +396,11 @@ class FunctionWrapper {
return js.withinHandleScope([&] {
auto context = js.v8Context();
v8::Local<v8::Value> argv[sizeof...(Args)]{
typeWrapper.wrap(context, kj::none, kj::fwd<Args>(args))...};
typeWrapper.wrap(js, context, kj::none, kj::fwd<Args>(args))...};

v8::Local<v8::Object> result = check(func->NewInstance(context, sizeof...(Args), argv));
return typeWrapper.template unwrap<Ret>(
context, result, TypeErrorContext::callbackReturn());
js, context, result, TypeErrorContext::callbackReturn());
});
};

Expand All @@ -404,15 +410,16 @@ class FunctionWrapper {
}

template <typename Ret, typename... Args>
kj::Maybe<Function<Ret(Args...)>> tryUnwrap(v8::Local<v8::Context> context,
kj::Maybe<Function<Ret(Args...)>> tryUnwrap(Lock& js,
v8::Local<v8::Context> context,
v8::Local<v8::Value> handle,
Function<Ret(Args...)>*,
kj::Maybe<v8::Local<v8::Object>> parentObject) {
if (!handle->IsFunction()) {
return kj::none;
}

auto isolate = context->GetIsolate();
auto isolate = js.v8Isolate;

auto wrapperFn = [](Lock& js, v8::Local<v8::Value> receiver, v8::Local<v8::Function> func,
Args... args) -> Ret {
Expand All @@ -423,13 +430,13 @@ class FunctionWrapper {
auto context = js.v8Context();
v8::LocalVector<v8::Value> argv(js.v8Isolate,
std::initializer_list<v8::Local<v8::Value>>{
typeWrapper.wrap(context, kj::none, kj::fwd<Args>(args))
typeWrapper.wrap(js, context, kj::none, kj::fwd<Args>(args))
.template As<v8::Value>()...});

auto result = check(func->Call(context, receiver, argv.size(), argv.data()));
if constexpr (!isVoid<Ret>()) {
return typeWrapper.template unwrap<Ret>(
context, result, TypeErrorContext::callbackReturn());
js, context, result, TypeErrorContext::callbackReturn());
} else {
return;
}
Expand All @@ -442,15 +449,16 @@ class FunctionWrapper {
}

template <typename Ret>
kj::Maybe<Function<Ret(Arguments<Value>)>> tryUnwrap(v8::Local<v8::Context> context,
kj::Maybe<Function<Ret(Arguments<Value>)>> tryUnwrap(Lock& js,
v8::Local<v8::Context> context,
v8::Local<v8::Value> handle,
Function<Ret(Arguments<Value>)>*,
kj::Maybe<v8::Local<v8::Object>> parentObject) {
if (!handle->IsFunction()) {
return kj::none;
}

auto isolate = context->GetIsolate();
auto isolate = js.v8Isolate;

auto wrapperFn = [](Lock& js, v8::Local<v8::Value> receiver, v8::Local<v8::Function> func,
Arguments<Value> args) -> Ret {
Expand All @@ -473,7 +481,7 @@ class FunctionWrapper {

if constexpr (!isVoid<Ret>()) {
return typeWrapper.template unwrap<Ret>(
context, result, TypeErrorContext::callbackReturn());
js, context, result, TypeErrorContext::callbackReturn());
} else {
return;
}
Expand Down
Loading
Loading