From e7dba04fdad149de62ec8404616e6cdc71cf2f9b Mon Sep 17 00:00:00 2001 From: Dan Lapid Date: Fri, 4 Oct 2024 16:25:50 +0000 Subject: [PATCH] Two context solution working as expected --- src/workerd/server/tests/python/pool-test.c++ | 242 +++++++++++------- 1 file changed, 151 insertions(+), 91 deletions(-) diff --git a/src/workerd/server/tests/python/pool-test.c++ b/src/workerd/server/tests/python/pool-test.c++ index b9e62d3cfe1..3ed7f13fd61 100644 --- a/src/workerd/server/tests/python/pool-test.c++ +++ b/src/workerd/server/tests/python/pool-test.c++ @@ -62,8 +62,10 @@ struct TestContext: public jsg::Object, public jsg::ContextGlobal { return jsg::alloc(); } - JSG_RESOURCE_TYPE(TestContext) { - JSG_METHOD(makeUnsafeEval); + JSG_RESOURCE_TYPE(TestContext, CompatibilityFlags::Reader flags) { + if (flags.getPythonWorkers()) { + JSG_METHOD(makeUnsafeEval); + } JSG_METHOD(makeTestApi); } }; @@ -72,12 +74,12 @@ struct CounterObject { jsg::Function getId; JSG_STRUCT(getId); }; -struct SimpleTestContext: public jsg::Object, public jsg::ContextGlobal { - JSG_RESOURCE_TYPE(SimpleTestContext) {} -}; +// struct SimpleTestContext: public jsg::Object, public jsg::ContextGlobal { +// JSG_RESOURCE_TYPE(SimpleTestContext) {} +// }; JSG_DECLARE_ISOLATE_TYPE(TestIsolate, TestContext, TestApi, CounterObject, EW_UNSAFE_ISOLATE_TYPES); -JSG_DECLARE_ISOLATE_TYPE(SimpleTestIsolate, SimpleTestContext, CounterObject); +// JSG_DECLARE_ISOLATE_TYPE(SimpleTestIsolate, SimpleTestContext, CounterObject); class Configuration { public: @@ -90,31 +92,36 @@ private: CompatibilityFlags::Reader& flags; }; -// This just creates a bundle with code that has our Counter class, essentially this should signify -// that we can create a class and hold a reference to it. -kj::Own initializeBundleModuleRegistry(const jsg::ResolveObserver& observer) { - ModuleRegistry::Builder builder(observer, ModuleRegistry::Builder::Options::ALLOW_FALLBACK); - ModuleBundle::BuiltinBuilder builtinBuilder(ModuleBundle::BuiltinBuilder::Type::BUILTIN_ONLY); - auto source = R"script( -class Counter { - constructor() { - this._counter = 0; - console.log(this._counter); - } +void expectEval( + jsg::Lock& js, kj::StringPtr code, kj::StringPtr expectedType, kj::StringPtr expectedValue) { + // Create a string containing the JavaScript source code. + v8::Local source = jsg::v8Str(js.v8Isolate, code); - getId() { - let val = ++this._counter; - console.log(val); - return val; + // Compile the source code. + v8::Local script; + if (!v8::Script::Compile(js.v8Context(), source).ToLocal(&script)) { + KJ_FAIL_EXPECT("code didn't parse", code); + return; } -} -export let counter = new Counter(); - )script"_kjc; - const auto specifier = "foo:bar"_url; - builtinBuilder.addEsm(specifier, source.slice(0, source.size()).attach(kj::mv(source))); - builder.add(builtinBuilder.finish()); - return builder.finish(); + v8::TryCatch catcher(js.v8Isolate); + + // Run the script to get the result. + v8::Local 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(js.v8Isolate, catcher.Exception()); + + KJ_EXPECT(expectedType == "throws", expectedType, catcher.Exception()); + KJ_EXPECT(*message == expectedValue, *message, expectedValue); + } else { + KJ_FAIL_EXPECT("returned empty handle but didn't throw exception?"); + } } // This test passes and surprisingly enough shows that the counter object can be reused in a new @@ -124,86 +131,139 @@ export let counter = new Counter(); // a reference to the counter class in that subcontext. KJ_TEST("Reuse an object created from another context 2") { auto observer = kj::atomicRefcounted(); - auto registry = initializeBundleModuleRegistry(*observer); - jsg::NewContextOptions options{.newModuleRegistry = *registry}; + auto code = R"script( +class Counter { + constructor() { + this._counter = 0; + console.log(this._counter); + } - SimpleTestIsolate isolate(v8System, kj::atomicAddRef(*observer)); + getId() { + let val = ++this._counter; + console.log(val); + return val; + } +} +new Counter() + )script"_kjc; + capnp::MallocMessageBuilder flagsArena; + auto flags = flagsArena.initRoot<::workerd::CompatibilityFlags>(); + auto flagsReader = flags.asReader(); + Configuration config(flagsReader); + TestIsolate isolate(v8System, config, kj::atomicAddRef(*observer)); kj::Maybe counter; - isolate.runInLockScope([&](SimpleTestIsolate::Lock& lock) { + isolate.runInLockScope([&](TestIsolate::Lock& lock) { lock.withinHandleScope([&]() -> auto { - jsg::JsContext context = lock.newContext(options); - v8::Local ctx = context.getHandle(lock); + v8::Local ctx = v8::Context::New(lock.v8Isolate); KJ_ASSERT(!ctx.IsEmpty(), "unable to enter invalid v8::Context"); v8::Context::Scope scope(ctx); - auto value1 = ModuleRegistry::resolve( - lock, "foo:bar", "counter"_kjc, ResolveContext::Type::BUILTIN_ONLY); - auto& wrapper = SimpleTestIsolate_TypeWrapper::from(lock.v8Isolate); - counter = wrapper.tryUnwrap(lock.v8Context(), value1, (CounterObject*)nullptr, kj::none); - auto& localcounter = KJ_ASSERT_NONNULL(counter); - KJ_ASSERT(localcounter.getId(lock) == 1); - KJ_ASSERT(localcounter.getId(lock) == 2); - KJ_ASSERT(localcounter.getId(lock) == 3); - }); - }); - isolate.runInLockScope([&](SimpleTestIsolate::Lock& lock) { - lock.withinHandleScope([&]() -> auto { - for (int i = 4; i < 30; ++i) { - jsg::JsContext context = lock.newContext(options); - v8::Local ctx = context.getHandle(lock); - KJ_ASSERT(!ctx.IsEmpty(), "unable to enter invalid v8::Context"); - v8::Context::Scope scope(ctx); + v8::Local source = jsg::v8Str(lock.v8Isolate, code); + // Compile the source code. + v8::Local script; + if (!v8::Script::Compile(lock.v8Context(), source).ToLocal(&script)) { + KJ_FAIL_EXPECT("code didn't parse", code); + return; + } + v8::TryCatch catcher(lock.v8Isolate); + lock.setAllowEval(true); + KJ_DEFER(lock.setAllowEval(false)); + // Run the script to get the result. + v8::Local result; + if (script->Run(lock.v8Context()).ToLocal(&result)) { + + auto& wrapper = TestIsolate_TypeWrapper::from(lock.v8Isolate); + counter = wrapper.tryUnwrap(lock.v8Context(), result, (CounterObject*)nullptr, kj::none); auto& localcounter = KJ_ASSERT_NONNULL(counter); - KJ_ASSERT(localcounter.getId(lock) == i); + KJ_ASSERT(localcounter.getId(lock) == 1); + KJ_ASSERT(localcounter.getId(lock) == 2); + KJ_ASSERT(localcounter.getId(lock) == 3); + } else if (catcher.HasCaught()) { + v8::String::Utf8Value message(lock.v8Isolate, catcher.Exception()); + KJ_FAIL_ASSERT(kj::str(*message)); + } else { + KJ_FAIL_EXPECT("returned empty handle but didn't throw exception?"); } }); }); -} - -// This test shows some idea for an implementation where we first create a Simple Isolate Type -// with only the api types that we require for emscripten initialization, do the setup then later -// "move" that isolate into a more elaborate isolate type and use that for the rest of the code flow. -KJ_TEST("??? 3") { - auto observer = kj::atomicRefcounted(); - auto registry = initializeBundleModuleRegistry(*observer); - jsg::NewContextOptions options{.newModuleRegistry = *registry}; - - SimpleTestIsolate isolate(v8System, kj::atomicAddRef(*observer)); - kj::Maybe counter; - isolate.runInLockScope([&](SimpleTestIsolate::Lock& lock) { + flags.setPythonWorkers(true); + isolate.runInLockScope([&](TestIsolate::Lock& lock) { lock.withinHandleScope([&]() -> auto { - jsg::JsContext context = lock.newContext(options); + jsg::JsContext context = lock.newContext(); v8::Local ctx = context.getHandle(lock); KJ_ASSERT(!ctx.IsEmpty(), "unable to enter invalid v8::Context"); v8::Context::Scope scope(ctx); - auto value1 = ModuleRegistry::resolve( - lock, "foo:bar", "counter"_kjc, ResolveContext::Type::BUILTIN_ONLY); - auto& wrapper = SimpleTestIsolate_TypeWrapper::from(lock.v8Isolate); - counter = wrapper.tryUnwrap(lock.v8Context(), value1, (CounterObject*)nullptr, kj::none); auto& localcounter = KJ_ASSERT_NONNULL(counter); - KJ_ASSERT(localcounter.getId(lock) == 1); - KJ_ASSERT(localcounter.getId(lock) == 2); - KJ_ASSERT(localcounter.getId(lock) == 3); + KJ_ASSERT(localcounter.getId(lock) == 4); + expectEval(lock, "makeUnsafeEval().eval('1+1')", "number", "2"); + expectEval(lock, "makeTestApi().test2()", "number", "2"); + expectEval(lock, "makeTestApi().test1()", "throws", + "TypeError: makeTestApi(...).test1 is not a function"); }); }); - capnp::MallocMessageBuilder flagsArena; - auto flags = flagsArena.initRoot<::workerd::CompatibilityFlags>(); - auto flagsReader = flags.asReader(); - Configuration config(flagsReader); - // TestIsolate newIsolate(kj::mv(isolate), config); // This syntax isn't implemented yet obviously. - // isolate.runInLockScope([&](SimpleTestIsolate::Lock& lock) { - // lock.withinHandleScope([&]() -> auto { - // for (int i = 4; i < 30; ++i) { - // jsg::JsContext context = lock.newContext(options); - // v8::Local ctx = context.getHandle(lock); - // KJ_ASSERT(!ctx.IsEmpty(), "unable to enter invalid v8::Context"); - // v8::Context::Scope scope(ctx); - - // auto& localcounter = KJ_ASSERT_NONNULL(counter); - // KJ_ASSERT(localcounter.getId(lock) == i); - // } - // }); - // }); } + +// KJ_ASSERT(result->IsObject()); +// auto object = result.As(); +// v8::Local fn; +// KJ_ASSERT(object->Get(lock.v8Context(), jsg::v8StrIntern(lock.v8Isolate, "getId")).ToLocal(&fn)); +// KJ_ASSERT(fn->IsFunction()); +// To continue down NoJSG path use function.h::tryUnwrap here +// This test shows some idea for an implementation where we first create a Simple Isolate Type +// with only the api types that we require for emscripten initialization, do the setup then later +// "move" that isolate into a more elaborate isolate type and use that for the rest of the code flow. +// KJ_TEST("??? 3") { +// auto observer = kj::atomicRefcounted(); +// auto registry = initializeBundleModuleRegistry(*observer); +// jsg::NewContextOptions options{.newModuleRegistry = *registry}; + +// SimpleTestIsolate isolate(v8System, kj::atomicAddRef(*observer)); +// kj::Maybe counter; +// isolate.runInLockScope([&](SimpleTestIsolate::Lock& lock) { +// lock.withinHandleScope([&]() -> auto { +// jsg::JsContext context = lock.newContext(options); +// v8::Local ctx = context.getHandle(lock); +// KJ_ASSERT(!ctx.IsEmpty(), "unable to enter invalid v8::Context"); +// v8::Context::Scope scope(ctx); + +// // auto value1 = ModuleRegistry::resolve( +// // lock, "foo:bar", "counter"_kjc, ResolveContext::Type::BUILTIN_ONLY); +// v8::Local source = jsg::v8Str(js.v8Isolate, code); + +// // Compile the source code. +// v8::Local script; +// if (!v8::Script::Compile(js.v8Context(), source).ToLocal(&script)) { +// KJ_FAIL_EXPECT("code didn't parse", code); +// return; +// } + +// auto& wrapper = SimpleTestIsolate_TypeWrapper::from(lock.v8Isolate); +// counter = wrapper.tryUnwrap(lock.v8Context(), value1, (CounterObject*)nullptr, kj::none); + +// auto& localcounter = KJ_ASSERT_NONNULL(counter); +// KJ_ASSERT(localcounter.getId(lock) == 1); +// KJ_ASSERT(localcounter.getId(lock) == 2); +// KJ_ASSERT(localcounter.getId(lock) == 3); +// }); +// }); +// capnp::MallocMessageBuilder flagsArena; +// auto flags = flagsArena.initRoot<::workerd::CompatibilityFlags>(); +// auto flagsReader = flags.asReader(); +// Configuration config(flagsReader); +// // TestIsolate newIsolate(kj::mv(isolate), config); // This syntax isn't implemented yet obviously. +// // isolate.runInLockScope([&](SimpleTestIsolate::Lock& lock) { +// // lock.withinHandleScope([&]() -> auto { +// // for (int i = 4; i < 30; ++i) { +// // jsg::JsContext context = lock.newContext(options); +// // v8::Local ctx = context.getHandle(lock); +// // KJ_ASSERT(!ctx.IsEmpty(), "unable to enter invalid v8::Context"); +// // v8::Context::Scope scope(ctx); + +// // auto& localcounter = KJ_ASSERT_NONNULL(counter); +// // KJ_ASSERT(localcounter.getId(lock) == i); +// // } +// // }); +// // }); +// }