Get DefaultPlatform from IsolateBase

Author: ketanhwr

src/workerd/jsg/jsg.c++

@@ -272,8 +272,8 @@ void Lock::terminateExecution() {
 }
 
 void Lock::pumpMessageLoop() {
-  auto& system = const_cast<jsg::V8System&>(this->getV8System());
-  while (v8::platform::PumpMessageLoop(&system.getDefaultPlatform(), v8Isolate)) {}
+  auto platform = IsolateBase::from(v8Isolate).getDefaultPlatform();
+  while (v8::platform::PumpMessageLoop(platform, v8Isolate)) {}
 }
 
 Name Lock::newSymbol(kj::StringPtr symbol) {

src/workerd/jsg/jsg.h

@@ -2721,9 +2721,6 @@ class Lock {
   // Retrieve the worker environment.
   virtual kj::Maybe<Value> getWorkerEnv() = 0;
 
- protected:
-  virtual const V8System& getV8System() = 0;
-
  private:
   // Mark the jsg::Lock as being disallowed from being passed as a parameter into
   // a kj promise coroutine. Note that this only blocks directly passing the Lock

src/workerd/jsg/setup.c++

@@ -335,10 +335,9 @@ static v8::Isolate* newIsolate(v8::Isolate::CreateParams&& params, v8::CppHeap*
 }
 }  // namespace
 
-IsolateBase::IsolateBase(const V8System& system,
-    v8::Isolate::CreateParams&& createParams,
-    kj::Own<IsolateObserver> observer)
-    : system(system),
+IsolateBase::IsolateBase(
+    V8System& system, v8::Isolate::CreateParams&& createParams, kj::Own<IsolateObserver> observer)
+    : defaultPlatform(&system.getDefaultPlatform()),
       cppHeap(newCppHeap(const_cast<V8PlatformWrapper*>(&system.platformWrapper))),
       ptr(newIsolate(kj::mv(createParams), cppHeap.release())),
       envAsyncContextKey(kj::refcounted<AsyncContextFrame::StorageKey>()),

src/workerd/jsg/setup.h

@@ -86,10 +86,6 @@ class IsolateBase {
  public:
   static IsolateBase& from(v8::Isolate* isolate);
 
-  auto& getV8System() {
-    return system;
-  }
-
   // Unwraps a JavaScript exception as a kj::Exception.
   virtual kj::Exception unwrapException(
       v8::Local<v8::Context> context, v8::Local<v8::Value> exception) = 0;
@@ -227,6 +223,10 @@ class IsolateBase {
     return *envAsyncContextKey;
   }
 
+  v8::Platform* getDefaultPlatform() {
+    return defaultPlatform;
+  }
+
  private:
   template <typename TypeWrapper>
   friend class Isolate;
@@ -259,7 +259,7 @@ class IsolateBase {
 
   using Item = kj::OneOf<v8::Global<v8::Data>, RefToDelete>;
 
-  const V8System& system;
+  v8::Platform* defaultPlatform;
   // TODO(cleanup): After v8 13.4 is fully released we can inline this into `newIsolate`
   //                and remove this member.
   std::unique_ptr<class v8::CppHeap> cppHeap;
@@ -333,7 +333,7 @@ class IsolateBase {
   // Maps instructions to source code locations.
   kj::TreeMap<uintptr_t, CodeBlockInfo> codeMap;
 
-  explicit IsolateBase(const V8System& system,
+  explicit IsolateBase(V8System& system,
       v8::Isolate::CreateParams&& createParams,
       kj::Own<IsolateObserver> observer);
   ~IsolateBase() noexcept(false);
@@ -455,7 +455,7 @@ class Isolate: public IsolateBase {
   // and should be instantiated with `instantiateTypeWrapper` before `newContext` is called on
   // a jsg::Lock of this Isolate.
   template <typename MetaConfiguration>
-  explicit Isolate(const V8System& system,
+  explicit Isolate(V8System& system,
       MetaConfiguration&& configuration,
       kj::Own<IsolateObserver> observer,
       v8::Isolate::CreateParams createParams = {},
@@ -468,7 +468,7 @@ class Isolate: public IsolateBase {
   }
 
   // Use this constructor when no wrappers have any required configuration.
-  explicit Isolate(const V8System& system,
+  explicit Isolate(V8System& system,
       kj::Own<IsolateObserver> observer,
       v8::Isolate::CreateParams createParams = {})
       : Isolate(system, nullptr, kj::mv(observer), kj::mv(createParams)) {}

src/workerd/server/tests/weakref/test.mjs

@@ -1,3 +1,9 @@
+/*
+This is a node.js script which handlers workerd lifecycle and sends multiple requests.
+FinalizationRegistry callbacks run after the exported handler returns, and wd-test,
+which only run tests within a single test() handler, are not enough to test this behaviour
+*/
+
 import { env } from 'node:process';
 import { beforeEach, afterEach, test } from 'node:test';
 import assert from 'node:assert';
@@ -6,12 +12,14 @@ import { WorkerdServerHarness } from '../server-harness.mjs';
 // Global that is reset for each test.
 let workerd;
 
-assert(
-  env.WORKERD_BINARY !== undefined,
+assert.notStrictEqual(
+  env.WORKERD_BINARY,
+  undefined,
   'You must set the WORKERD_BINARY environment variable.'
 );
-assert(
-  env.WORKERD_CONFIG !== undefined,
+assert.notStrictEqual(
+  env.WORKERD_CONFIG,
+  undefined,
   'You must set the WORKERD_CONFIG environment variable.'
 );
 
@@ -33,7 +41,7 @@ beforeEach(async () => {
 // Stop workerd.
 afterEach(async () => {
   const [code, signal] = await workerd.stop();
-  assert(code === 0 || signal === 'SIGTERM');
+  assert(code === 0 || signal === 'SIGTERM', `code=${code}, signal=${signal}`);
   workerd = null;
 });
 
@@ -43,6 +51,6 @@ test('JS FinalizationRegistry', async () => {
   let httpPort = await workerd.getListenPort('http');
   for (let i = 0; i < 3; ++i) {
     const response = await fetch(`http://localhost:${httpPort}`);
-    assert.equal(await response.text(), i);
+    assert.strictEqual(await response.text(), `${i}`);
   }
 });