Skip to content
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.

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

JSG Completion: use getHandle(jsg::Lock&) consistently #1003

Merged
merged 1 commit into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/workerd/api/actor-state-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ KJ_TEST("v8 serialization version tag hasn't changed") {
jsg::V8StackScope stackScope;
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock.v8Isolate);
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

auto trueVal = v8::True(isolateLock.v8Isolate);
Expand Down Expand Up @@ -66,7 +66,7 @@ KJ_TEST("we support deserializing up to v15") {
jsg::V8StackScope stackScope;
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock.v8Isolate);
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

kj::Vector<kj::StringPtr> testCases;
Expand Down Expand Up @@ -115,7 +115,7 @@ KJ_TEST("wire format version does not change deserialization behavior on real da
jsg::V8StackScope stackScope;
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock.v8Isolate);
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

// Read in data line by line and verify that it round trips (serializes and then deserializes)
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/crypto-impl-aes-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ KJ_TEST("AES-CTR key wrap") {
auto isolate = isolateLock.v8Isolate;

isolateLock.withinHandleScope([&] {
auto context = isolateLock.newContext<CryptoContext>().getHandle(isolate);
auto context = isolateLock.newContext<CryptoContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(context);

auto& js = jsg::Lock::from(isolate);
Expand Down
9 changes: 4 additions & 5 deletions src/workerd/api/gpu/gpu-buffer.c++
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,17 @@ GPUBuffer::getMappedRange(jsg::Lock& js, jsg::Optional<GPUSize64> offset,

v8::Local<v8::ArrayBuffer> arrayBuffer =
v8::ArrayBuffer::New(js.v8Isolate, backing);
arrayBuffer->SetDetachKey(detachKey_.getHandle(js.v8Isolate));
arrayBuffer->SetDetachKey(detachKey_.getHandle(js));

mapped_.add(Mapping{start, end,
jsg::V8Ref<v8::ArrayBuffer>(js.v8Isolate, arrayBuffer)});
mapped_.add(Mapping{ start, end, js.v8Ref(arrayBuffer) });
return arrayBuffer;
}

void GPUBuffer::DetachMappings(jsg::Lock& js) {
for (auto& mapping : mapped_) {
auto ab = mapping.buffer.getHandle(js.v8Isolate);
auto ab = mapping.buffer.getHandle(js);

auto res = ab->Detach(detachKey_.getHandle(js.v8Isolate));
auto res = ab->Detach(detachKey_.getHandle(js));
KJ_ASSERT(res.IsJust());
}
mapped_.clear();
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/http.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1408,7 +1408,7 @@ kj::Maybe<jsg::Ref<WebSocket>> Response::getWebSocket(jsg::Lock& js) {
jsg::Optional<v8::Local<v8::Object>> Response::getCf(
const v8::PropertyCallbackInfo<v8::Value>& info) {
return cf.map([&](jsg::V8Ref<v8::Object>& handle) {
return handle.getHandle(info.GetIsolate());
return handle.getHandle(jsg::Lock::from(info.GetIsolate()));
});
}

Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/kv.c++
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,9 @@ jsg::Promise<jsg::Value> KvNamespace::list(jsg::Lock& js, jsg::Optional<ListOpti
.attach(kj::mv(stream)),
[cacheStatus = kj::mv(cacheStatus)](jsg::Lock& js, kj::String text) mutable {
auto result = js.parseJson(text);
parseListMetadata(js, result.getHandle(js.v8Isolate),
parseListMetadata(js, result.getHandle(js),
cacheStatus.map([&](jsg::Value& cs) -> v8::Local<v8::Value> {
return cs.getHandle((js.v8Isolate));
return cs.getHandle(js);
}));
return result;
});
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/queue.c++
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ jsg::Ref<QueueEvent> startQueueEvent(

KJ_IF_MAYBE(h, exportedHandler) {
auto queueHandler = KJ_ASSERT_NONNULL(handlerHandler.tryUnwrap(
lock, h->self.getHandle(lock.getIsolate())));
lock, h->self.getHandle(lock)));
KJ_IF_MAYBE(f, queueHandler.queue) {
auto promise = (*f)(lock, jsg::alloc<QueueController>(event.addRef()),
h->env.addRef(js), h->getCtx());
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/sockets.c++
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ jsg::Promise<void> Socket::maybeCloseWriteSide(jsg::Lock& js) {
// been flushed.
return writable->getController().close(js).catch_(js,
JSG_VISITABLE_LAMBDA((ref=JSG_THIS), (ref), (jsg::Lock& js, jsg::Value&& exc) {
ref->closedResolver.reject(js, exc.getHandle(js.v8Isolate));
ref->closedResolver.reject(js, exc.getHandle(js));
})).then(js, JSG_VISITABLE_LAMBDA((ref=JSG_THIS), (ref), (jsg::Lock& js) {
ref->closedResolver.resolve(js);
}));
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/sockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class Socket: public jsg::Object {
};

jsg::Promise<void> errorHandler(jsg::Lock& js, jsg::Value err) {
auto jsException = err.getHandle(js.v8Isolate);
auto jsException = err.getHandle(js);
resolveFulfiller(js, jsg::createTunneledException(js.v8Isolate, jsException));
return js.resolvedPromise();
};
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/streams/queue-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct Preamble {
: isolate(v8System, kj::heap<jsg::IsolateObserver>()),
lock(isolate, stackScope),
scope(lock.v8Isolate),
context(lock.newContext<QueueContext>().getHandle(lock.v8Isolate)),
context(lock.newContext<QueueContext>().getHandle(lock)),
contextScope(context) {}

jsg::Lock& getJs() { return jsg::Lock::from(lock.v8Isolate); }
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/streams/standard.c++
Original file line number Diff line number Diff line change
Expand Up @@ -3669,7 +3669,7 @@ jsg::Promise<void> WritableStreamJsController::pipeLoop(jsg::Lock& js) {
};

auto promise = write(js, result.value.map([&](jsg::Value& value) {
return value.getHandle(js.v8Isolate);
return value.getHandle(js);
}));

return maybeAddFunctor(js, kj::mv(promise), kj::mv(onSuccess), kj::mv(onFailure));
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/io-context.c++
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ void IoContext::logUncaughtExceptionAsync(UncaughtExceptionSource source,
jsg::Lock& js = lock;
auto error = js.exceptionToJs(kj::mv(exception));
// TODO(soon): Add logUncaughtException to jsg::Lock.
lock.logUncaughtException(source, error.getHandle(js.v8Isolate));
lock.logUncaughtException(source, error.getHandle(js));
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/io-context.h
Original file line number Diff line number Diff line change
Expand Up @@ -1416,14 +1416,14 @@ kj::_::ReducePromises<RemoveIoOwn<T>> IoContext::awaitJs(jsg::Lock& js, jsg::Pro

auto errorHandler =
[fulfiller = addObject(kj::addRef(*fulfiller))]
(jsg::Lock&, jsg::Value jsExceptionRef) mutable {
(jsg::Lock& js, jsg::Value jsExceptionRef) mutable {
// Note: `context` can possibly be different than the one that started the wait, if the
// promise resolved from a different context. In that case the use of `fulfiller` will
// throw later on. But it's OK to use the wrong context up until that point.
auto& context = IoContext::current();

auto isolate = context.getCurrentLock().getIsolate();
auto jsException = jsExceptionRef.getHandle(isolate);
auto jsException = jsExceptionRef.getHandle(js);

// TODO(someday): We log an "uncaught exception" here whenever a promise returned from JS to
// C++ rejects. However, the C++ code waiting on the promise may do its own logging (e.g.
Expand Down
10 changes: 5 additions & 5 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ struct Worker::Isolate::Impl {
lock->withinHandleScope([&] {
context->clear();
KJ_IF_MAYBE(i, impl.inspector) {
i->get()->contextDestroyed(context.getHandle(lock->v8Isolate));
i->get()->contextDestroyed(context.getHandle(*lock));
}
{ auto drop = kj::mv(context); }
lock->v8Isolate->ContextDisposedNotification(false);
Expand Down Expand Up @@ -1213,7 +1213,7 @@ Worker::Script::Script(kj::Own<const Isolate> isolateParam, kj::StringPtr id,
// Modules can't be compiled for multiple contexts. We need to create the real context now.
auto& mContext = impl->moduleContext.emplace(isolate->apiIsolate->newContext(lock));
mContext->enableWarningOnSpecialEvents();
context = mContext.getHandle(lock.v8Isolate);
context = mContext.getHandle(lock);
recordedLock.setupContext(context);
} else {
// Although we're going to compile a script independent of context, V8 requires that there be
Expand Down Expand Up @@ -1424,7 +1424,7 @@ Worker::Worker(kj::Own<const Script> scriptParam,
jsContext = &this->impl->context.emplace(script->isolate->apiIsolate->newContext(lock));
}

v8::Local<v8::Context> context = KJ_REQUIRE_NONNULL(jsContext).getHandle(lock.v8Isolate);
v8::Local<v8::Context> context = KJ_REQUIRE_NONNULL(jsContext).getHandle(lock);
if (!script->modular) {
recordedLock.setupContext(context);
}
Expand Down Expand Up @@ -1716,9 +1716,9 @@ v8::Isolate* Worker::Lock::getIsolate() {

v8::Local<v8::Context> Worker::Lock::getContext() {
KJ_IF_MAYBE(c, worker.impl->context) {
return c->getHandle(impl->inner.v8Isolate);
return c->getHandle(impl->inner);
} else KJ_IF_MAYBE(c, const_cast<Script&>(*worker.script).impl->moduleContext) {
return c->getHandle(impl->inner.v8Isolate);
return c->getHandle(impl->inner);
} else {
KJ_UNREACHABLE;
}
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/buffersource.c++
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ BackingStore BufferSource::detach(Lock& js) {

bool BufferSource::canDetach(Lock& js) {
if (isDetached()) return false;
return isDetachable(handle.getHandle(js.v8Isolate));
return isDetachable(handle.getHandle(js));
}

v8::Local<v8::Value> BufferSource::getHandle(Lock& js) {
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/buffersource.h
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ class BufferSource {
if (isDetached()) {
return nullptr;
}
auto h = handle.getHandle(js.v8Isolate);
auto h = getHandle(js);
if (h->IsArrayBuffer()) {
return h.As<v8::ArrayBuffer>()->ByteLength();
} else if (h->IsArrayBufferView()) {
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/function-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ struct FunctionContext: public ContextGlobalObject {
return js.tryCatch([&]() {
return kj::str(thrower(js));
}, [&](Value exception) {
auto handle = exception.getHandle(js.v8Isolate);
auto handle = exception.getHandle(js);
return kj::str("caught: ", handle);
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/jsg/function.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ class Function<Ret(Args...)> {
return (*native)(jsl, kj::fwd<Args>(args)...);
}
KJ_CASE_ONEOF(js, JsImpl) {
return (*js.wrapper)(jsl, js.receiver.getHandle(jsl.v8Isolate),
js.handle.getHandle(jsl.v8Isolate), kj::fwd<Args>(args)...);
return (*js.wrapper)(jsl, js.receiver.getHandle(jsl),
js.handle.getHandle(jsl), kj::fwd<Args>(args)...);
}
}
__builtin_unreachable();
Expand Down
12 changes: 9 additions & 3 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -1406,6 +1406,7 @@ class JsContext {
v8::Local<v8::Context> getHandle(v8::Isolate* isolate) {
return handle.Get(isolate);
}
v8::Local<v8::Context> getHandle(Lock& js);

private:
v8::Global<v8::Context> handle;
Expand Down Expand Up @@ -2130,7 +2131,7 @@ inline v8::Local<v8::Value> Lock::v8Null() {
}

inline Data Data::addRef(jsg::Lock& js) {
return Data(js.v8Isolate, getHandle(js.v8Isolate));
return Data(js.v8Isolate, getHandle(js));
}

template <typename T>
Expand All @@ -2140,7 +2141,7 @@ kj::Maybe<v8::Local<v8::Object>> Ref<T>::tryGetHandle(Lock& js) {

template <typename T>
inline V8Ref<T> V8Ref<T>::addRef(jsg::Lock& js) {
return js.v8Ref(getHandle(js.v8Isolate));
return js.v8Ref(getHandle(js));
}

v8::Local<v8::Value> deepClone(v8::Local<v8::Context> context, v8::Local<v8::Value> value);
Expand All @@ -2154,7 +2155,7 @@ V8Ref<T> V8Ref<T>::deepClone(jsg::Lock& js) {

template <typename T>
inline HashableV8Ref<T> HashableV8Ref<T>::addRef(jsg::Lock& js) {
return HashableV8Ref(js.v8Isolate, this->getHandle(js.v8Isolate), identityHash);
return HashableV8Ref(js.v8Isolate, this->getHandle(js), identityHash);
}

template <typename T>
Expand All @@ -2166,6 +2167,11 @@ inline v8::Local<v8::Data> Data::getHandle(jsg::Lock& js) {
return getHandle(js.v8Isolate);
}

template <typename T>
inline v8::Local<v8::Context> JsContext<T>::getHandle(Lock& js) {
return handle.Get(js.v8Isolate);
}

} // namespace workerd::jsg

// These two includes are needed for the JSG type glue macros to work.
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/jsg/promise-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ struct PromiseContext: public jsg::Object, public jsg::ContextGlobal {

void catchIt(jsg::Lock& js, Promise<int> promise) {
promise.catch_(js, [](jsg::Lock& js, Value value) -> int {
JSG_FAIL_REQUIRE(Error, kj::str(value.getHandle(js.v8Isolate)));
JSG_FAIL_REQUIRE(Error, kj::str(value.getHandle(js)));
}).then(js, [](jsg::Lock& js, int i) {
KJ_FAIL_REQUIRE("shouldn't get here");
return kj::str("nope");
}, [](jsg::Lock& js, Value value) {
return kj::str(value.getHandle(js.v8Isolate));
return kj::str(value.getHandle(js));
}).then(js, [](jsg::Lock&, kj::String s) {
catchTestResult = kj::mv(s);
});
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/promise.c++
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void UnhandledRejectionHandler::rejectedWithNoHandler(
jsg::Lock& js,
jsg::V8Ref<v8::Promise> promise,
jsg::V8Ref<v8::Value> value) {
auto message = v8::Exception::CreateMessage(js.v8Isolate, value.getHandle(js.v8Isolate));
auto message = v8::Exception::CreateMessage(js.v8Isolate, value.getHandle(js));

// It's not yet clear under what conditions it happens, but this can be called
// twice with the same promise. It really shouldn't happen in the regular cases
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/jsg/promise.h
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ class Promise {
}

Resolver addRef(Lock& js) {
return { js.v8Isolate, v8Resolver.getHandle(js.v8Isolate) };
return { js.v8Isolate, v8Resolver.getHandle(js) };
}
void visitForGc(GcVisitor& visitor) {
visitor.visit(v8Resolver);
Expand Down Expand Up @@ -422,7 +422,7 @@ class Promise {

v8::Local<v8::Promise> getInner(Lock& js) {
return KJ_REQUIRE_NONNULL(v8Promise, "jsg::Promise can only be used once")
.getHandle(js.v8Isolate);
.getHandle(js);
}

template <typename U = T, typename = kj::EnableIf<!isVoid<U>()>()>
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/jsg/setup-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ KJ_TEST("configuration values reach nested type declarations") {
V8StackScope stackScope;
ConfigIsolate::Lock lock(isolate, stackScope);
v8::HandleScope handleScope(lock.v8Isolate);
lock.newContext<ConfigContext>().getHandle(lock.v8Isolate);
lock.newContext<ConfigContext>().getHandle(lock);
}
{
KJ_EXPECT_LOG(ERROR, "failed: expected configuration == 123");
Expand All @@ -79,7 +79,7 @@ KJ_TEST("configuration values reach nested type declarations") {
V8StackScope stackScope;
ConfigIsolate::Lock lock(isolate, stackScope);
v8::HandleScope handleScope(lock.v8Isolate);
lock.newContext<ConfigContext>().getHandle(lock.v8Isolate);
lock.newContext<ConfigContext>().getHandle(lock);
}
}

Expand Down
Loading