diff --git a/src/workerd/io/observer.h b/src/workerd/io/observer.h index b4043921341..134b074837e 100644 --- a/src/workerd/io/observer.h +++ b/src/workerd/io/observer.h @@ -141,7 +141,9 @@ class RequestObserver: public kj::Refcounted { } }; -class IsolateObserver: public kj::AtomicRefcounted, public jsg::IsolateObserver { +class JsgIsolateObserver: public kj::AtomicRefcounted, public jsg::IsolateObserver {}; + +class IsolateObserver: public kj::AtomicRefcounted { public: virtual ~IsolateObserver() noexcept(false) {} diff --git a/src/workerd/io/worker.c++ b/src/workerd/io/worker.c++ index d3148ce9feb..a40a39d0577 100644 --- a/src/workerd/io/worker.c++ +++ b/src/workerd/io/worker.c++ @@ -663,7 +663,7 @@ struct Worker::Isolate::Impl { // because our GlobalScope object needs to have a function called on it, and any attached // inspector needs to be notified. JSG doesn't know about these things. - Impl(Api& api, + Impl(const Api& api, IsolateObserver& metrics, IsolateLimitEnforcer& limitEnforcer, InspectorPolicy inspectorPolicy) @@ -971,20 +971,22 @@ const HeapSnapshotDeleter HeapSnapshotDeleter::INSTANCE; } // namespace Worker::Isolate::Isolate(kj::Own apiParam, + kj::Own metricsParam, kj::StringPtr id, kj::Own limitEnforcerParam, InspectorPolicy inspectorPolicy, ConsoleMode consoleMode) - : metrics(kj::atomicAddRef(apiParam->getMetrics())), + : metrics(kj::mv(metricsParam)), id(kj::str(id)), - api(kj::mv(apiParam)), limitEnforcer(kj::mv(limitEnforcerParam)), + api(kj::mv(apiParam)), consoleMode(consoleMode), featureFlagsForFl(makeCompatJson(decompileCompatibilityFlagsForFl(api->getFeatureFlags()))), impl(kj::heap(*api, *metrics, *limitEnforcer, inspectorPolicy)), weakIsolateRef(WeakIsolateRef::wrap(this)), traceAsyncContextKey(kj::refcounted()) { api->setEnforcer(*limitEnforcer); + api->setIsolateObserver(*metrics); metrics->created(); // We just created our isolate, so we don't need to use Isolate::Impl::Lock (nor an async lock). jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) { @@ -1329,7 +1331,7 @@ Worker::Script::Script(kj::Own isolateParam, KJ_SWITCH_ONEOF(source) { KJ_CASE_ONEOF(script, ScriptSource) { impl->globals = - script.compileGlobals(lock, isolate->getApi(), isolate->impl->metrics); + script.compileGlobals(lock, isolate->getApi(), isolate->getApi().getObserver()); { // It's unclear to me if CompileUnboundScript() can get trapped in any infinite loops or @@ -1398,6 +1400,7 @@ Worker::Isolate::~Isolate() noexcept(false) { auto dropTraceAsyncContextKey = kj::mv(traceAsyncContextKey); }); api->invalidateEnforcer(); + api->invalidateIsolateObserver(); } Worker::Script::~Script() noexcept(false) { diff --git a/src/workerd/io/worker.h b/src/workerd/io/worker.h index 0dfadfc5c81..24691578e90 100644 --- a/src/workerd/io/worker.h +++ b/src/workerd/io/worker.h @@ -275,6 +275,7 @@ class Worker::Isolate: public kj::AtomicRefcounted { // session has one isolate which may load many iterations of the script (this allows the // inspector session to stay open across them). explicit Isolate(kj::Own api, + kj::Own metrics, kj::StringPtr id, kj::Own limitEnforcer, InspectorPolicy inspectorPolicy, @@ -409,9 +410,8 @@ class Worker::Isolate: public kj::AtomicRefcounted { TeardownFinishedGuard teardownGuard{*metrics}; kj::String id; - kj::Own api; - // TODO: should this be before or after api? kj::Own limitEnforcer; + kj::Own api; ConsoleMode consoleMode; // If non-null, a serialized JSON object with a single "flags" property, which is a list of @@ -534,8 +534,9 @@ class Worker::Api { virtual jsg::JsObject wrapExecutionContext( jsg::Lock& lock, jsg::Ref ref) const = 0; - virtual IsolateObserver& getMetrics() = 0; - virtual const IsolateObserver& getMetrics() const = 0; + virtual const jsg::IsolateObserver& getObserver() const = 0; + virtual void setIsolateObserver(IsolateObserver&) = 0; + virtual void invalidateIsolateObserver() = 0; virtual void setEnforcer(IsolateLimitEnforcer&) = 0; virtual void invalidateEnforcer() = 0; diff --git a/src/workerd/jsg/disable-memoize-test.c++ b/src/workerd/jsg/disable-memoize-test.c++ index fe1750b2c95..9bce9e63d96 100644 --- a/src/workerd/jsg/disable-memoize-test.c++ +++ b/src/workerd/jsg/disable-memoize-test.c++ @@ -137,7 +137,7 @@ void expectEval( } KJ_TEST("Create a context with memoization disabled change flags then create another context") { - auto observer = kj::atomicRefcounted(); + auto observer = kj::atomicRefcounted(); capnp::MallocMessageBuilder flagsArena; auto flags = flagsArena.initRoot<::workerd::CompatibilityFlags>(); auto flagsReader = flags.asReader(); diff --git a/src/workerd/server/server.c++ b/src/workerd/server/server.c++ index e948aa37c72..5f6c6cad422 100644 --- a/src/workerd/server/server.c++ +++ b/src/workerd/server/server.c++ @@ -2928,6 +2928,7 @@ kj::Own Server::makeWorker(kj::StringPtr name, } }; + auto jsgobserver = kj::atomicRefcounted(); auto observer = kj::atomicRefcounted(); auto limitEnforcer = kj::refcounted(); @@ -2937,19 +2938,19 @@ kj::Own Server::makeWorker(kj::StringPtr name, "The new ModuleRegistry implementation is an experimental feature. " "You must run workerd with `--experimental` to use this feature."); newModuleRegistry = WorkerdApi::initializeBundleModuleRegistry( - *observer, conf, featureFlags.asReader(), pythonConfig); + *jsgobserver, conf, featureFlags.asReader(), pythonConfig); } auto api = kj::heap(globalContext->v8System, featureFlags.asReader(), - limitEnforcer->getCreateParams(), kj::mv(observer), *memoryCacheProvider, pythonConfig, + limitEnforcer->getCreateParams(), kj::mv(jsgobserver), *memoryCacheProvider, pythonConfig, kj::mv(newModuleRegistry)); auto inspectorPolicy = Worker::Isolate::InspectorPolicy::DISALLOW; if (inspectorOverride != kj::none) { // For workerd, if the inspector is enabled, it is always fully trusted. inspectorPolicy = Worker::Isolate::InspectorPolicy::ALLOW_FULLY_TRUSTED; } - auto isolate = kj::atomicRefcounted(kj::mv(api), name, kj::mv(limitEnforcer), - inspectorPolicy, + auto isolate = kj::atomicRefcounted(kj::mv(api), kj::mv(observer), name, + kj::mv(limitEnforcer), inspectorPolicy, conf.isServiceWorkerScript() ? Worker::ConsoleMode::INSPECTOR_ONLY : consoleMode); // If we are using the inspector, we need to register the Worker::Isolate diff --git a/src/workerd/server/workerd-api.c++ b/src/workerd/server/workerd-api.c++ index 1540af83884..801849f328b 100644 --- a/src/workerd/server/workerd-api.c++ +++ b/src/workerd/server/workerd-api.c++ @@ -131,7 +131,7 @@ static const PythonConfig defaultConfig{ struct WorkerdApi::Impl final { kj::Own features; kj::Maybe> maybeOwnedModuleRegistry; - kj::Own observer; + kj::Own observer; JsgWorkerdIsolate jsgIsolate; api::MemoryCacheProvider& memoryCacheProvider; const PythonConfig& pythonConfig; @@ -159,7 +159,7 @@ struct WorkerdApi::Impl final { Impl(jsg::V8System& v8System, CompatibilityFlags::Reader featuresParam, v8::Isolate::CreateParams createParams, - kj::Own observerParam, + kj::Own observerParam, api::MemoryCacheProvider& memoryCacheProvider, const PythonConfig& pythonConfig = defaultConfig, kj::Maybe> newModuleRegistry = kj::none) @@ -168,10 +168,7 @@ struct WorkerdApi::Impl final { observer(kj::atomicAddRef(*observerParam)), jsgIsolate(v8System, Configuration(*this), kj::mv(observerParam), kj::mv(createParams)), memoryCacheProvider(memoryCacheProvider), - pythonConfig(pythonConfig) { - jsgIsolate.runInLockScope( - [&](JsgWorkerdIsolate::Lock& lock) { limitEnforcer->customizeIsolate(lock.v8Isolate); }); - } + pythonConfig(pythonConfig) {} static v8::Local compileTextGlobal( JsgWorkerdIsolate::Lock& lock, capnp::Text::Reader reader) { @@ -214,7 +211,7 @@ struct WorkerdApi::Impl final { WorkerdApi::WorkerdApi(jsg::V8System& v8System, CompatibilityFlags::Reader features, v8::Isolate::CreateParams createParams, - kj::Own observer, + kj::Own observer, api::MemoryCacheProvider& memoryCacheProvider, const PythonConfig& pythonConfig, kj::Maybe> newModuleRegistry) @@ -270,14 +267,12 @@ jsg::JsObject WorkerdApi::wrapExecutionContext( kj::downcast(lock).wrap(lock.v8Context(), kj::mv(ref))); } -IsolateObserver& WorkerdApi::getMetrics() { - return *impl->observer; -} - -const IsolateObserver& WorkerdApi::getMetrics() const { +const jsg::IsolateObserver& WorkerdApi::getObserver() const { return *impl->observer; } +void WorkerdApi::setIsolateObserver(IsolateObserver&) {}; +void WorkerdApi::invalidateIsolateObserver() {}; void WorkerdApi::setEnforcer(IsolateLimitEnforcer&) {} void WorkerdApi::invalidateEnforcer() {} diff --git a/src/workerd/server/workerd-api.h b/src/workerd/server/workerd-api.h index a465eb1283a..1a9b36420ca 100644 --- a/src/workerd/server/workerd-api.h +++ b/src/workerd/server/workerd-api.h @@ -35,7 +35,7 @@ class WorkerdApi final: public Worker::Api { WorkerdApi(jsg::V8System& v8System, CompatibilityFlags::Reader features, v8::Isolate::CreateParams createParams, - kj::Own observer, + kj::Own observer, api::MemoryCacheProvider& memoryCacheProvider, const PythonConfig& pythonConfig, kj::Maybe> newModuleRegistry); @@ -55,8 +55,9 @@ class WorkerdApi final: public Worker::Api { jsg::Lock& lock) const override; jsg::JsObject wrapExecutionContext( jsg::Lock& lock, jsg::Ref ref) const override; - IsolateObserver& getMetrics() override; - const IsolateObserver& getMetrics() const override; + const jsg::IsolateObserver& getObserver() const override; + void setIsolateObserver(IsolateObserver&) override; + void invalidateIsolateObserver() override; void setEnforcer(IsolateLimitEnforcer&) override; void invalidateEnforcer() override; diff --git a/src/workerd/tests/test-fixture.c++ b/src/workerd/tests/test-fixture.c++ index 2f4cfac5fee..694743c86a5 100644 --- a/src/workerd/tests/test-fixture.c++ +++ b/src/workerd/tests/test-fixture.c++ @@ -324,11 +324,12 @@ TestFixture::TestFixture(SetupParams&& params) api(kj::heap(testV8System, params.featureFlags.orDefault(CompatibilityFlags::Reader()), kj::heap()->getCreateParams(), - kj::atomicRefcounted(), + kj::atomicRefcounted(), *memoryCacheProvider, defaultPythonConfig, kj::none)), workerIsolate(kj::atomicRefcounted(kj::mv(api), + kj::atomicRefcounted(), scriptId, kj::heap(), Worker::Isolate::InspectorPolicy::DISALLOW)),