Refactor observer to split JsgIsolateObserver and IsolateObserver

This will allow for better separation of responsibilities between the two.

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) {}
 

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,21 @@ const HeapSnapshotDeleter HeapSnapshotDeleter::INSTANCE;
 }  // namespace
 
 Worker::Isolate::Isolate(kj::Own<Api> apiParam,
+    kj::Own<IsolateObserver> metricsParam,
     kj::StringPtr id,
     kj::Own<IsolateLimitEnforcer> 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<Impl>(*api, *metrics, *limitEnforcer, inspectorPolicy)),
       weakIsolateRef(WeakIsolateRef::wrap(this)),
       traceAsyncContextKey(kj::refcounted<jsg::AsyncContextFrame::StorageKey>()) {
-  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 +1330,7 @@ Worker::Script::Script(kj::Own<const Isolate> 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
@@ -1402,7 +1403,6 @@ Worker::Isolate::~Isolate() noexcept(false) {
     auto inspector = kj::mv(impl->inspector);
     auto dropTraceAsyncContextKey = kj::mv(traceAsyncContextKey);
   });
-  api->invalidateEnforcer();
 }
 
 Worker::Script::~Script() noexcept(false) {

src/workerd/io/worker.h

@@ -274,7 +274,11 @@ class Worker::Isolate: public kj::AtomicRefcounted {
   // Usually it matches the script ID. An exception is preview isolates: there, each preview
   // session has one isolate which may load many iterations of the script (this allows the
   // inspector session to stay open across them).
+  // The Isolate object owns the Api object and outlives it in order to report teardown timing.
+  // The Api object is created before the Isolate object and does not strictly require
+  // request-specific information.
   explicit Isolate(kj::Own<Api> api,
+      kj::Own<IsolateObserver> metrics,
       kj::StringPtr id,
       kj::Own<IsolateLimitEnforcer> limitEnforcer,
       InspectorPolicy inspectorPolicy,
@@ -409,9 +413,8 @@ class Worker::Isolate: public kj::AtomicRefcounted {
   TeardownFinishedGuard<IsolateObserver&> teardownGuard{*metrics};
 
   kj::String id;
-  kj::Own<Api> api;
-  // TODO: should this be before or after api?
   kj::Own<IsolateLimitEnforcer> limitEnforcer;
+  kj::Own<Api> api;
   ConsoleMode consoleMode;
 
   // If non-null, a serialized JSON object with a single "flags" property, which is a list of
@@ -534,10 +537,8 @@ class Worker::Api {
   virtual jsg::JsObject wrapExecutionContext(
       jsg::Lock& lock, jsg::Ref<api::ExecutionContext> ref) const = 0;
 
-  virtual IsolateObserver& getMetrics() = 0;
-  virtual const IsolateObserver& getMetrics() const = 0;
-  virtual void setEnforcer(IsolateLimitEnforcer&) = 0;
-  virtual void invalidateEnforcer() = 0;
+  virtual const jsg::IsolateObserver& getObserver() const = 0;
+  virtual void setIsolateObserver(IsolateObserver&) = 0;
 
   // Set the module fallback service callback, if any.
   using ModuleFallbackCallback = kj::Maybe<kj::OneOf<kj::String, jsg::ModuleRegistry::ModuleInfo>>(

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<workerd::IsolateObserver>();
+  auto observer = kj::atomicRefcounted<JsgIsolateObserver>();
   capnp::MallocMessageBuilder flagsArena;
   auto flags = flagsArena.initRoot<::workerd::CompatibilityFlags>();
   auto flagsReader = flags.asReader();

src/workerd/server/server.c++

@@ -2921,6 +2921,7 @@ kj::Own<Server::Service> Server::makeWorker(kj::StringPtr name,
     }
   };
 
+  auto jsgobserver = kj::atomicRefcounted<JsgIsolateObserver>();
   auto observer = kj::atomicRefcounted<IsolateObserver>();
   auto limitEnforcer = kj::refcounted<NullIsolateLimitEnforcer>();
 
@@ -2930,19 +2931,19 @@ kj::Own<Server::Service> 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<WorkerdApi>(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<Worker::Isolate>(kj::mv(api), name, kj::mv(limitEnforcer),
-      inspectorPolicy,
+  auto isolate = kj::atomicRefcounted<Worker::Isolate>(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

src/workerd/server/workerd-api.c++

@@ -132,7 +132,7 @@ static const PythonConfig defaultConfig{
 struct WorkerdApi::Impl final {
   kj::Own<CompatibilityFlags::Reader> features;
   kj::Maybe<kj::Own<jsg::modules::ModuleRegistry>> maybeOwnedModuleRegistry;
-  kj::Own<IsolateObserver> observer;
+  kj::Own<JsgIsolateObserver> observer;
   JsgWorkerdIsolate jsgIsolate;
   api::MemoryCacheProvider& memoryCacheProvider;
   const PythonConfig& pythonConfig;
@@ -160,7 +160,7 @@ struct WorkerdApi::Impl final {
   Impl(jsg::V8System& v8System,
       CompatibilityFlags::Reader featuresParam,
       v8::Isolate::CreateParams createParams,
-      kj::Own<IsolateObserver> observerParam,
+      kj::Own<JsgIsolateObserver> observerParam,
       api::MemoryCacheProvider& memoryCacheProvider,
       const PythonConfig& pythonConfig = defaultConfig,
       kj::Maybe<kj::Own<jsg::modules::ModuleRegistry>> newModuleRegistry = kj::none)
@@ -169,10 +169,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<v8::String> compileTextGlobal(
       JsgWorkerdIsolate::Lock& lock, capnp::Text::Reader reader) {
@@ -215,7 +212,7 @@ struct WorkerdApi::Impl final {
 WorkerdApi::WorkerdApi(jsg::V8System& v8System,
     CompatibilityFlags::Reader features,
     v8::Isolate::CreateParams createParams,
-    kj::Own<IsolateObserver> observer,
+    kj::Own<JsgIsolateObserver> observer,
     api::MemoryCacheProvider& memoryCacheProvider,
     const PythonConfig& pythonConfig,
     kj::Maybe<kj::Own<jsg::modules::ModuleRegistry>> newModuleRegistry)
@@ -271,16 +268,11 @@ jsg::JsObject WorkerdApi::wrapExecutionContext(
       kj::downcast<JsgWorkerdIsolate::Lock>(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::setEnforcer(IsolateLimitEnforcer&) {}
-void WorkerdApi::invalidateEnforcer() {}
+void WorkerdApi::setIsolateObserver(IsolateObserver&) {};
 
 Worker::Script::Source WorkerdApi::extractSource(kj::StringPtr name,
     config::Worker::Reader conf,

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<IsolateObserver> observer,
+      kj::Own<JsgIsolateObserver> observer,
       api::MemoryCacheProvider& memoryCacheProvider,
       const PythonConfig& pythonConfig,
       kj::Maybe<kj::Own<jsg::modules::ModuleRegistry>> newModuleRegistry);
@@ -55,10 +55,8 @@ class WorkerdApi final: public Worker::Api {
       jsg::Lock& lock) const override;
   jsg::JsObject wrapExecutionContext(
       jsg::Lock& lock, jsg::Ref<api::ExecutionContext> ref) const override;
-  IsolateObserver& getMetrics() override;
-  const IsolateObserver& getMetrics() const override;
-  void setEnforcer(IsolateLimitEnforcer&) override;
-  void invalidateEnforcer() override;
+  const jsg::IsolateObserver& getObserver() const override;
+  void setIsolateObserver(IsolateObserver&) override;
 
   static Worker::Script::Source extractSource(kj::StringPtr name,
       config::Worker::Reader conf,

src/workerd/tests/test-fixture.c++

@@ -324,11 +324,12 @@ TestFixture::TestFixture(SetupParams&& params)
       api(kj::heap<server::WorkerdApi>(testV8System,
           params.featureFlags.orDefault(CompatibilityFlags::Reader()),
           kj::heap<MockIsolateLimitEnforcer>()->getCreateParams(),
-          kj::atomicRefcounted<IsolateObserver>(),
+          kj::atomicRefcounted<JsgIsolateObserver>(),
           *memoryCacheProvider,
           defaultPythonConfig,
           kj::none)),
       workerIsolate(kj::atomicRefcounted<Worker::Isolate>(kj::mv(api),
+          kj::atomicRefcounted<IsolateObserver>(),
           scriptId,
           kj::heap<MockIsolateLimitEnforcer>(),
           Worker::Isolate::InspectorPolicy::DISALLOW)),