Skip to content

Commit d9a4ed0

Browse files
committed
Add workerd test and run format
1 parent 2954bd5 commit d9a4ed0

File tree

11 files changed

+140
-27
lines changed

11 files changed

+140
-27
lines changed

Diff for: src/workerd/io/io-context.c++

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
#include <workerd/io/io-gate.h>
88
#include <workerd/io/worker.h>
99
#include <workerd/jsg/jsg.h>
10+
#include <workerd/jsg/setup.h>
1011
#include <workerd/util/sentry.h>
1112
#include <workerd/util/uncaught-exception-source.h>
12-
#include <workerd/jsg/setup.h>
1313

1414
#include <kj/debug.h>
1515

@@ -451,7 +451,8 @@ kj::Promise<void> IoContext::IncomingRequest::drain() {
451451
context->pumpMessageLoop();
452452
return context->waitUntilTasks.onEmpty()
453453
.exclusiveJoin(kj::mv(timeoutPromise))
454-
.exclusiveJoin(context->abortPromise.addBranch().then([] {}, [](kj::Exception&&) {}));
454+
.exclusiveJoin(context->abortPromise.addBranch().then(
455+
[] {}, [](kj::Exception&& e) { KJ_LOG(INFO, "uncaught exception", e); }));
455456
}
456457

457458
kj::Promise<IoContext_IncomingRequest::FinishScheduledResult> IoContext::IncomingRequest::

Diff for: src/workerd/jsg/jsg.c++

+1-2
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@
44

55
#include "jsg.h"
66

7+
#include "libplatform/libplatform.h"
78
#include "setup.h"
89

910
#include <workerd/jsg/util.h>
1011
#include <workerd/util/thread-scopes.h>
1112

12-
#include "libplatform/libplatform.h"
13-
1413
namespace workerd::jsg {
1514

1615
kj::String stringifyHandle(v8::Local<v8::Value> value) {

Diff for: src/workerd/jsg/jsg.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -2715,7 +2715,7 @@ class Lock {
27152715
// Returns the env base object.
27162716
virtual JsObject getEnv(bool release = false) = 0;
27172717

2718-
protected:
2718+
protected:
27192719
virtual const V8System& getV8System() = 0;
27202720

27212721
private:

Diff for: src/workerd/jsg/setup.c++

+11-6
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,19 @@ static kj::Own<v8::Platform> userPlatform(v8::Platform& platform) {
7777
V8System::V8System(): V8System(defaultPlatform(0), nullptr) {}
7878
V8System::V8System(kj::ArrayPtr<const kj::StringPtr> flags): V8System(defaultPlatform(0), flags) {}
7979
V8System::V8System(v8::Platform& platformParam): V8System(platformParam, nullptr) {}
80-
V8System::V8System(v8::Platform& platformParam, kj::ArrayPtr<const kj::StringPtr> flags, v8::Platform* defaultPlatformPtr)
80+
V8System::V8System(v8::Platform& platformParam,
81+
kj::ArrayPtr<const kj::StringPtr> flags,
82+
v8::Platform* defaultPlatformPtr)
8183
: V8System(userPlatform(platformParam), flags, defaultPlatformPtr) {}
82-
V8System::V8System(kj::Own<v8::Platform> platformParam, kj::ArrayPtr<const kj::StringPtr> flags, v8::Platform* defaultPlatformPtr)
84+
V8System::V8System(kj::Own<v8::Platform> platformParam,
85+
kj::ArrayPtr<const kj::StringPtr> flags,
86+
v8::Platform* defaultPlatformPtr)
8387
: platformInner(kj::mv(platformParam)),
84-
platformWrapper(*platformInner), defaultPlatformPtr_{defaultPlatformPtr} {
85-
if (!defaultPlatformPtr_) {
86-
defaultPlatformPtr_ = platformInner.get();
87-
}
88+
platformWrapper(*platformInner),
89+
defaultPlatformPtr_{defaultPlatformPtr} {
90+
if (!defaultPlatformPtr_) {
91+
defaultPlatformPtr_ = platformInner.get();
92+
}
8893
#if V8_HAS_STACK_START_MARKER
8994
v8::StackStartMarker::EnableForProcess();
9095
#endif

Diff for: src/workerd/jsg/setup.h

+14-7
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,28 @@ class V8System {
5656
explicit V8System(v8::Platform& platform);
5757

5858
// Use a possibly-custom v8::Platform implementation, and apply flags.
59-
explicit V8System(v8::Platform& platform, kj::ArrayPtr<const kj::StringPtr> flags, v8::Platform* defaultPlatformPtr = nullptr);
59+
explicit V8System(v8::Platform& platform,
60+
kj::ArrayPtr<const kj::StringPtr> flags,
61+
v8::Platform* defaultPlatformPtr = nullptr);
6062

6163
~V8System() noexcept(false);
6264

6365
typedef void FatalErrorCallback(kj::StringPtr location, kj::StringPtr message);
6466
static void setFatalErrorCallback(FatalErrorCallback* callback);
6567

66-
auto& getDefaultPlatform() { return *defaultPlatformPtr_; }
68+
auto& getDefaultPlatform() {
69+
return *defaultPlatformPtr_;
70+
}
6771

6872
private:
6973
kj::Own<v8::Platform> platformInner;
7074
V8PlatformWrapper platformWrapper;
7175
friend class IsolateBase;
7276
v8::Platform* defaultPlatformPtr_;
7377

74-
explicit V8System(kj::Own<v8::Platform>, kj::ArrayPtr<const kj::StringPtr>, v8::Platform* defaultPlatformPtr = nullptr);
78+
explicit V8System(kj::Own<v8::Platform>,
79+
kj::ArrayPtr<const kj::StringPtr>,
80+
v8::Platform* defaultPlatformPtr = nullptr);
7581
};
7682

7783
// Base class of Isolate<T> containing parts that don't need to be templated, to avoid code
@@ -687,10 +693,6 @@ class Isolate: public IsolateBase {
687693
*this, value.toString(*this), value, JsMessage::create(*this, value));
688694
}
689695
}
690-
protected:
691-
virtual const V8System& getV8System() override {
692-
return jsgIsolate.getV8System();
693-
}
694696

695697
// Sets an env value that will be expressed on the process.env
696698
// if/when nodejs-compat mode is used.
@@ -710,6 +712,11 @@ class Isolate: public IsolateBase {
710712
return JsObject(jsgIsolate.envObj.Get(v8Isolate));
711713
}
712714

715+
protected:
716+
virtual const V8System& getV8System() override {
717+
return jsgIsolate.getV8System();
718+
}
719+
713720
private:
714721
Isolate& jsgIsolate;
715722

Diff for: src/workerd/server/tests/weakref/BUILD.bazel

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
load("@aspect_rules_js//js:defs.bzl", "js_test")
2+
3+
js_test(
4+
name = "js-weak-ref-test",
5+
data = [
6+
":config.capnp",
7+
":index.mjs",
8+
"//src/workerd/server:workerd",
9+
"//src/workerd/server/tests:server-harness_js_lib",
10+
],
11+
entry_point = "test.mjs",
12+
env = {
13+
"WORKERD_BINARY": "$(rootpath //src/workerd/server:workerd)",
14+
"WORKERD_CONFIG": "$(rootpath :config.capnp)",
15+
},
16+
tags = ["js-test"],
17+
)

Diff for: src/workerd/server/tests/weakref/config.capnp

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# config.capnp
2+
using Workerd = import "/workerd/workerd.capnp";
3+
4+
const config :Workerd.Config = (
5+
v8Flags = [ "--expose-gc" ],
6+
services = [
7+
( name = "main", worker = .worker ),
8+
],
9+
sockets = [
10+
( name = "http", address = "*:0", http = (), service = "main" ),
11+
]
12+
);
13+
14+
const worker :Workerd.Worker = (
15+
modules = [
16+
( name = "./index.mjs", esModule = embed "index.mjs" )
17+
],
18+
compatibilityDate = "2024-01-01",
19+
compatibilityFlags = ["enable_weak_ref"],
20+
);
21+

Diff for: src/workerd/server/tests/weakref/index.mjs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
let counter = 0;
2+
const fr = new FinalizationRegistry(() => {
3+
++counter;
4+
});
5+
6+
export default {
7+
async fetch(request, env, ctx) {
8+
(function () {
9+
let obj = {};
10+
fr.register(obj, '');
11+
obj = undefined;
12+
})();
13+
14+
// Ensure obj gets GC'd
15+
gc();
16+
17+
return new Response(counter);
18+
},
19+
};

Diff for: src/workerd/server/tests/weakref/test.mjs

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { env } from 'node:process';
2+
import { beforeEach, afterEach, test } from 'node:test';
3+
import assert from 'node:assert';
4+
import { WorkerdServerHarness } from '../server-harness.mjs';
5+
6+
// Global that is reset for each test.
7+
let workerd;
8+
9+
assert(
10+
env.WORKERD_BINARY !== undefined,
11+
'You must set the WORKERD_BINARY environment variable.'
12+
);
13+
assert(
14+
env.WORKERD_CONFIG !== undefined,
15+
'You must set the WORKERD_CONFIG environment variable.'
16+
);
17+
18+
// Start workerd.
19+
beforeEach(async () => {
20+
workerd = new WorkerdServerHarness({
21+
workerdBinary: env.WORKERD_BINARY,
22+
workerdConfig: env.WORKERD_CONFIG,
23+
24+
// Hard-coded to match a socket name expected in the `workerdConfig` file.
25+
listenPortNames: ['http'],
26+
});
27+
28+
await workerd.start();
29+
30+
await workerd.getListenPort('http');
31+
});
32+
33+
// Stop workerd.
34+
afterEach(async () => {
35+
const [code, signal] = await workerd.stop();
36+
assert(code === 0 || signal === 'SIGTERM');
37+
workerd = null;
38+
});
39+
40+
// FinalizationRegistry callbacks only run after the exported handler returns
41+
// So we should see its effects in a follow-up request
42+
test('JS FinalizationRegistry', async () => {
43+
let httpPort = await workerd.getListenPort('http');
44+
for (let i = 0; i < 3; ++i) {
45+
const response = await fetch(`http://localhost:${httpPort}`);
46+
assert.equal(await response.text(), i);
47+
}
48+
});

Diff for: src/workerd/server/workerd-api.c++

+3-7
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,7 @@ struct WorkerdApi::Impl final {
271271
auto version = getPythonBundleName(pythonRelease);
272272
auto bundle = KJ_ASSERT_NONNULL(
273273
fetchPyodideBundle(pythonConfig, version), "Failed to get Pyodide bundle");
274-
jsg::NewContextOptions options{
275-
.enableWeakRef = features->getJsWeakRef()
276-
};
274+
jsg::NewContextOptions options{.enableWeakRef = features->getJsWeakRef()};
277275
auto context = lock.newContext<api::ServiceWorkerGlobalScope>(options, lock.v8Isolate);
278276
v8::Context::Scope scope(context.getHandle(lock));
279277
// Init emscripten synchronously, the python script will import setup-emscripten and
@@ -344,10 +342,8 @@ CompatibilityFlags::Reader WorkerdApi::getFeatureFlags() const {
344342
return *impl->features;
345343
}
346344
jsg::JsContext<api::ServiceWorkerGlobalScope> WorkerdApi::newContext(jsg::Lock& lock) const {
347-
jsg::NewContextOptions options{
348-
.newModuleRegistry = impl->tryGetModuleRegistry(),
349-
.enableWeakRef = getFeatureFlags().getJsWeakRef()
350-
};
345+
jsg::NewContextOptions options{.newModuleRegistry = impl->tryGetModuleRegistry(),
346+
.enableWeakRef = getFeatureFlags().getJsWeakRef()};
351347
return kj::downcast<JsgWorkerdIsolate::Lock>(lock).newContext<api::ServiceWorkerGlobalScope>(
352348
kj::mv(options), lock.v8Isolate);
353349
}

Diff for: src/workerd/server/workerd.c++

+2-2
Original file line numberDiff line numberDiff line change
@@ -1335,8 +1335,8 @@ class CliMain final: public SchemaFileImpl::ErrorReporter {
13351335
auto config = getConfig();
13361336
auto platform = jsg::defaultPlatform(0);
13371337
WorkerdPlatform v8Platform(*platform);
1338-
jsg::V8System v8System(
1339-
v8Platform, KJ_MAP(flag, config.getV8Flags()) -> kj::StringPtr { return flag; }, platform.get());
1338+
jsg::V8System v8System(v8Platform,
1339+
KJ_MAP(flag, config.getV8Flags()) -> kj::StringPtr { return flag; }, platform.get());
13401340
auto promise = func(v8System, config);
13411341
KJ_IF_SOME(w, watcher) {
13421342
promise = promise.exclusiveJoin(waitForChanges(w).then([this]() {

0 commit comments

Comments
 (0)