Skip to content

Commit

Permalink
Merge pull request #1575 from cloudflare/jsnell/context-scope-part-2
Browse files Browse the repository at this point in the history
  • Loading branch information
@jasnell
jasnell authored Jan 25, 2024
2 parents 123a429 + 775a8c2 commit 0a957b4
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 51 deletions.
7 changes: 3 additions & 4 deletions src/workerd/api/streams/queue-test.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ JSG_DECLARE_ISOLATE_TYPE(QueueIsolate, QueueContext);
void preamble(auto callback) {
QueueIsolate isolate(v8System, kj::heap<jsg::IsolateObserver>());
isolate.runInLockScope([&](QueueIsolate::Lock& lock) {
lock.withinHandleScope([&] {
v8::Local<v8::Context> context = lock.newContext<QueueContext>().getHandle(lock);
v8::Context::Scope contextScope(context);
jsg::Lock& js = lock;
JSG_WITHIN_CONTEXT_SCOPE(lock,
lock.newContext<QueueContext>().getHandle(lock),
[&](jsg::Lock& js) {
callback(js);
});
});
Expand Down
4 changes: 1 addition & 3 deletions src/workerd/io/io-context.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -990,9 +990,7 @@ void IoContext::runInContextScope(
currentInputLock = kj::mv(inputLock);
currentLock = lock;

jsg::Lock& js = lock;
js.withinHandleScope([&] {
v8::Context::Scope contextScope(lock.getContext());
JSG_WITHIN_CONTEXT_SCOPE(lock, lock.getContext(), [&](jsg::Lock& js) {
v8::Isolate::PromiseContextScope promiseContextScope(
lock.getIsolate(), getPromiseContextTag(lock));

Expand Down
62 changes: 26 additions & 36 deletions src/workerd/jsg/jsg-test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,44 +31,40 @@ class Evaluator {
kj::StringPtr expectedType,
kj::StringPtr expectedValue) {
getIsolate().runInLockScope([&](typename IsolateType::Lock& lock) {
jsg::Lock& js = lock;
js.withinHandleScope([&] {
// Create a new context.
auto jsContext = lock.template newContext<ContextType>();
v8::Local<v8::Context> context = jsContext.getHandle(lock.v8Isolate);

// Enter the context for the module
v8::Context::Scope contextScope(context);

JSG_WITHIN_CONTEXT_SCOPE(lock,
lock.template newContext<ContextType>().getHandle(lock.v8Isolate),
[&](jsg::Lock& js) {
// Compile code as "main" module
CompilationObserver observer;
auto modules = ModuleRegistryImpl<ContextType>::from(lock);
auto modules = ModuleRegistryImpl<ContextType>::from(js);
auto p = kj::Path::parse("main");
modules->add(p, jsg::ModuleRegistry::ModuleInfo(
lock, "main", code, ModuleInfoCompileOption::BUNDLE, observer));

// Instantiate the module
auto& moduleInfo = KJ_REQUIRE_NONNULL(modules->resolve(lock, p));
auto module = moduleInfo.module.getHandle(lock);
jsg::instantiateModule(lock, module);
auto& moduleInfo = KJ_REQUIRE_NONNULL(modules->resolve(js, p));
auto module = moduleInfo.module.getHandle(js);
jsg::instantiateModule(js, module);

// Module has to export "run" function
auto moduleNs = check(module->GetModuleNamespace()->ToObject(context));
auto runValue = check(moduleNs->Get(context, v8StrIntern(lock.v8Isolate, "run"_kj)));
auto moduleNs = check(module->GetModuleNamespace()->ToObject(js.v8Context()));
auto runValue = check(moduleNs->Get(js.v8Context(), v8StrIntern(lock.v8Isolate, "run"_kj)));

v8::TryCatch catcher(lock.v8Isolate);
v8::TryCatch catcher(js.v8Isolate);

// Run the function to get the result.
v8::Local<v8::Value> result;
if (v8::Function::Cast(*runValue)->Call(context, context->Global(), 0, nullptr)
if (v8::Function::Cast(*runValue)->Call(
js.v8Context(),
js.v8Context()->Global(), 0, nullptr)
.ToLocal(&result)) {
v8::String::Utf8Value type(lock.v8Isolate, result->TypeOf(lock.v8Isolate));
v8::String::Utf8Value value(lock.v8Isolate, result);
v8::String::Utf8Value type(js.v8Isolate, result->TypeOf(js.v8Isolate));
v8::String::Utf8Value value(js.v8Isolate, result);

KJ_EXPECT(*type == expectedType, *type, expectedType);
KJ_EXPECT(*value == expectedValue, *value, expectedValue);
} else if (catcher.HasCaught()) {
v8::String::Utf8Value message(lock.v8Isolate, catcher.Exception());
v8::String::Utf8Value message(js.v8Isolate, catcher.Exception());

KJ_EXPECT(expectedType == "throws", expectedType, catcher.Exception());
KJ_EXPECT(*message == expectedValue, *message, expectedValue);
Expand All @@ -81,37 +77,31 @@ class Evaluator {

void expectEval(kj::StringPtr code, kj::StringPtr expectedType, kj::StringPtr expectedValue) {
getIsolate().runInLockScope([&](typename IsolateType::Lock& lock) {
jsg::Lock& js = lock;
js.withinHandleScope([&] {
// Create a new context.
v8::Local<v8::Context> context =
lock.template newContext<ContextType>().getHandle(lock.v8Isolate);

// Enter the context for compiling and running the hello world script.
v8::Context::Scope contextScope(context);

JSG_WITHIN_CONTEXT_SCOPE(lock,
lock.template newContext<ContextType>().getHandle(lock.v8Isolate),
[&](jsg::Lock& js) {
// Create a string containing the JavaScript source code.
v8::Local<v8::String> source = jsg::v8Str(lock.v8Isolate, code);
v8::Local<v8::String> source = jsg::v8Str(js.v8Isolate, code);

// Compile the source code.
v8::Local<v8::Script> script;
if (!v8::Script::Compile(context, source).ToLocal(&script)) {
if (!v8::Script::Compile(js.v8Context(), source).ToLocal(&script)) {
KJ_FAIL_EXPECT("code didn't parse", code);
return;
}

v8::TryCatch catcher(lock.v8Isolate);
v8::TryCatch catcher(js.v8Isolate);

// Run the script to get the result.
v8::Local<v8::Value> result;
if (script->Run(context).ToLocal(&result)) {
v8::String::Utf8Value type(lock.v8Isolate, result->TypeOf(lock.v8Isolate));
v8::String::Utf8Value value(lock.v8Isolate, result);
if (script->Run(js.v8Context()).ToLocal(&result)) {
v8::String::Utf8Value type(js.v8Isolate, result->TypeOf(js.v8Isolate));
v8::String::Utf8Value value(js.v8Isolate, result);

KJ_EXPECT(*type == expectedType, *type, expectedType);
KJ_EXPECT(*value == expectedValue, *value, expectedValue);
} else if (catcher.HasCaught()) {
v8::String::Utf8Value message(lock.v8Isolate, catcher.Exception());
v8::String::Utf8Value message(js.v8Isolate, catcher.Exception());

KJ_EXPECT(expectedType == "throws", expectedType, catcher.Exception());
KJ_EXPECT(*message == expectedValue, *message, expectedValue);
Expand Down
16 changes: 8 additions & 8 deletions src/workerd/jsg/resource.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1227,13 +1227,14 @@ class ResourceWrapper {
// Expose the type of the global scope in the global scope itself.
exposeGlobalScopeType(isolate, context);

auto moduleRegistry = ModuleRegistryImpl<TypeWrapper>::install(isolate, context, compilationObserver);
auto moduleRegistry = ModuleRegistryImpl<TypeWrapper>::install(
isolate, context, compilationObserver);
ptr->setModuleRegistry(kj::mv(moduleRegistry));

v8::Context::Scope context_scope(context);
setupJavascript(js, context);

return JsContext<T>(context, kj::mv(ptr));
return JSG_WITHIN_CONTEXT_SCOPE(js, context, [&](jsg::Lock& js) {
setupJavascript(js);
return JsContext<T>(context, kj::mv(ptr));
});
}

kj::Maybe<T&> tryUnwrap(
Expand Down Expand Up @@ -1322,9 +1323,8 @@ class ResourceWrapper {
return scope.Escape(constructor);
}

void setupJavascript(jsg::Lock& js, v8::Local<v8::Context> context) {
v8::Context::Scope context_scope(context);
JsSetup<TypeWrapper, T> setup(js, context);
void setupJavascript(jsg::Lock& js) {
JsSetup<TypeWrapper, T> setup(js, js.v8Context());

if constexpr (isDetected<GetConfiguration, T>()) {
T::template registerMembers<decltype(setup), T>(setup, configuration);
Expand Down

0 comments on commit 0a957b4

Please sign in to comment.