JSRPC: Use Symbol.dispose instead of dispose().

This polyfills `Symbol.dispose` and `Symbol.asyncDispose` per the explicit resource management spec:

https://github.com/tc39/proposal-explicit-resource-management

I decided to entirely replace the special name "dispose" with Symbol.dispose, rather than write complicated logic to make it possible to use either one. However, I could be convinced that this is too ugly and that we should go back to allowing "dispose" as an alias.

src/workerd/api/tests/js-rpc-test.js

@@ -17,7 +17,7 @@ class MyCounter extends RpcTarget {
   #disposedResolver;
   #onDisposedPromise = new Promise(resolve => this.#disposedResolver = resolve);
 
-  dispose() {
+  [Symbol.dispose]() {
     this.disposed = true;
     this.#disposedResolver();
   }
@@ -209,7 +209,7 @@ export class MyService extends WorkerEntrypoint {
     result.value = dupCallback.foo;
 
     // Not really
-    result.dispose = () => dupCallback.dispose();
+    result[Symbol.dispose] = () => dupCallback[Symbol.dispose]();
 
     return result;
   }
@@ -241,8 +241,8 @@ export class MyService extends WorkerEntrypoint {
         // This will succeed since we kept a dup.
         return await counterDup.increment(n);
       },
-      dispose() {
-        counterDup.dispose();
+      [Symbol.dispose]() {
+        counterDup[Symbol.dispose]();
       }
     };
   }
@@ -556,7 +556,7 @@ export let loopbackJsRpcTarget = {
     assert.strictEqual(await stub.increment(7), 16);
 
     assert.strictEqual(counter.disposed, false);
-    stub.dispose();
+    stub[Symbol.dispose]();
 
     await assert.rejects(stub.increment(2), {
       name: "Error",
@@ -605,7 +605,7 @@ export let disposal = {
     {
       let counter = await env.MyService.makeCounter(12)
       assert.strictEqual(await counter.increment(3), 15);
-      counter.dispose();
+      counter[Symbol.dispose]();
       await assert.rejects(counter.increment(2), {
         name: "Error",
         message: "RPC stub used after being disposed."
@@ -618,7 +618,7 @@ export let disposal = {
       let obj = await env.MyService.getAnObject(5);
       assert.strictEqual(await obj.counter.increment(7), 12);
 
-      obj.dispose();
+      obj[Symbol.dispose]();
       await assert.rejects(obj.counter.increment(2), {
         name: "Error",
         message: "RPC stub used after being disposed."
@@ -654,7 +654,7 @@ export let disposal = {
       assert.strictEqual(await obj.counter.increment(4), 19);
 
       // But of course, disposing the return value overall breaks everything.
-      obj.dispose();
+      obj[Symbol.dispose]();
       await assert.rejects(obj.counter.increment(2), {
         name: "Error",
         message: "RPC stub used after being disposed."
@@ -751,8 +751,8 @@ export let crossContextSharingDoesntWork = {
 }
 
 function stripDispose(obj) {
-  assert.deepEqual(!!obj.dispose, true);
-  delete obj.dispose;
+  assert.deepEqual(!!obj[Symbol.dispose], true);
+  delete obj[Symbol.dispose];
   return obj;
 }
 

src/workerd/api/worker-rpc.c++

@@ -116,7 +116,7 @@ jsg::JsValue deserializeRpcReturnValue(jsg::Lock& js,
           (jsg::Lock&, const v8::FunctionCallbackInfo<v8::Value>&) mutable {
         disposalGroup->disposeAll();
       });
-      obj.set(js, "dispose", jsg::JsValue(func));
+      obj.set(js, js.symbolDispose(), jsg::JsValue(func));
     }
   } else {
     // Result wasn't an object, so it must not contain any stubs.
@@ -190,7 +190,7 @@ private:
 void tryCallDisposeMethod(jsg::Lock& js, jsg::JsValue value) {
   js.withinHandleScope([&]() {
     KJ_IF_SOME(obj, value.tryCast<jsg::JsObject>()) {
-      auto dispose = obj.get(js, "dispose");
+      auto dispose = obj.get(js, js.symbolDispose());
       if (dispose.isFunction()) {
         jsg::check(v8::Local<v8::Value>(dispose).As<v8::Function>()->Call(
             js.v8Context(), value, 0, nullptr));
@@ -915,7 +915,6 @@ private:
         name == "webSocketMessage" ||
         name == "webSocketClose" ||
         name == "webSocketError" ||
-        name == "dispose" ||
         name == "dup" ||
         // All JS classes define a method `constructor` on the prototype, but we don't actually
         // want this to be callable over RPC!
@@ -1177,7 +1176,7 @@ public:
         pendingEvent(ioCtx.registerPendingEvent()) {
     // Check for the existence of a dispose function now so that the destructor doesn't have to
     // take an isolate lock if there isn't one.
-    auto getResult = object.get(js, "dispose");
+    auto getResult = object.get(js, js.symbolDispose());
     if (getResult.isFunction()) {
       dispose.emplace(js.v8Isolate, v8::Local<v8::Value>(getResult).As<v8::Function>());
     }
@@ -1250,14 +1249,14 @@ static MakeCallPipeline::Result makeCallPipeline(jsg::Lock& js, jsg::JsValue val
     if (obj.getPrototype() == js.obj().getPrototype()) {
       // It's a plain object.
       kj::Maybe<v8::Local<v8::Function>> maybeDispose;
-      jsg::JsValue disposeProperty = obj.get(js, "dispose");
+      jsg::JsValue disposeProperty = obj.get(js, js.symbolDispose());
       if (disposeProperty.isFunction()) {
         maybeDispose = v8::Local<v8::Value>(disposeProperty).As<v8::Function>();
       }
 
       // We don't want the disposer to be serialized, so delete it from the object. (Remember
       // that a new `dispose()` method will always be added on the client side).
-      obj.delete_(js, "dispose");
+      obj.delete_(js, js.symbolDispose());
 
       return MakeCallPipeline::Object {
         .cap = rpc::JsRpcTarget::Client(kj::heap<TransientJsRpcTarget>(

src/workerd/api/worker-rpc.h

@@ -160,7 +160,7 @@ class JsRpcPromise: public JsRpcClientProvider {
   kj::Maybe<jsg::Ref<JsRpcProperty>> getProperty(jsg::Lock& js, kj::String name);
 
   JSG_RESOURCE_TYPE(JsRpcPromise) {
-    JSG_METHOD(dispose);
+    JSG_DISPOSE(dispose);
     JSG_CALLABLE(call);
     JSG_WILDCARD_PROPERTY(getProperty);
     JSG_METHOD(then);
@@ -304,7 +304,7 @@ class JsRpcStub: public JsRpcClientProvider {
 
   JSG_RESOURCE_TYPE(JsRpcStub) {
     JSG_METHOD(dup);
-    JSG_METHOD(dispose);
+    JSG_DISPOSE(dispose);
     JSG_CALLABLE(call);
     JSG_WILDCARD_PROPERTY(getRpcMethod);
   }

src/workerd/jsg/jsg.c++

@@ -263,6 +263,13 @@ Name Lock::newApiSymbol(kj::StringPtr symbol) {
   return Name(*this, v8::Symbol::ForApi(v8Isolate, v8StrIntern(v8Isolate, symbol)));
 }
 
+JsSymbol Lock::symbolDispose() {
+  return IsolateBase::from(v8Isolate).getSymbolDispose();
+}
+JsSymbol Lock::symbolAsyncDispose() {
+  return IsolateBase::from(v8Isolate).getSymbolAsyncDispose();
+}
+
 Name::Name(kj::String string)
     : hash(kj::hashCode(string)),
       inner(kj::mv(string)) {}

src/workerd/jsg/jsg.h

@@ -251,6 +251,17 @@ namespace workerd::jsg {
     registry.template registerAsyncIterable<NAME, decltype(&Self::method), &Self::method>(); \
   } while (false)
 
+#define JSG_DISPOSE(method) \
+  do { \
+    static const char NAME[] = #method; \
+    registry.template registerDispose<NAME, decltype(&Self::method), &Self::method>(); \
+  } while (false)
+#define JSG_ASYNC_DISPOSE(method) \
+  do { \
+    static const char NAME[] = #method; \
+    registry.template registerAsyncDispose<NAME, decltype(&Self::method), &Self::method>(); \
+  } while (false)
+
 // Use inside a JSG_RESOURCE_TYPE block to declare a property on this object that should be
 // accessible to JavaScript. `name` is the JavaScript member name, while `getter` and `setter` are
 // the names of C++ methods that get and set this property.
@@ -2367,6 +2378,8 @@ class Lock {
 #define V(Name) JsSymbol symbol##Name() KJ_WARN_UNUSED_RESULT;
   JS_V8_SYMBOLS(V)
 #undef V
+  JsSymbol symbolDispose() KJ_WARN_UNUSED_RESULT;
+  JsSymbol symbolAsyncDispose() KJ_WARN_UNUSED_RESULT;
 
   void runMicrotasks();
   void terminateExecution();

src/workerd/jsg/resource.c++

@@ -3,6 +3,7 @@
 //     https://opensource.org/licenses/Apache-2.0
 
 #include "jsg.h"
+#include "setup.h"
 
 namespace workerd::jsg {
 
@@ -24,6 +25,28 @@ void exposeGlobalScopeType(v8::Isolate* isolate, v8::Local<v8::Context> context)
   KJ_ASSERT(check(global->Set(context, name, constructor)));
 }
 
+void polyfillSymbols(jsg::Lock& js, v8::Local<v8::Context> context) {
+  js.withinHandleScope([&]() {
+    JsObject obj(context->Global());
+
+    auto symbol = KJ_ASSERT_NONNULL(obj.get(js, "Symbol").tryCast<JsObject>());
+
+    KJ_ASSERT(!symbol.has(js, "dispose") && !symbol.has(js, "asyncDispose"),
+        "It looks like V8 has been updated to support the explicit resource management spec! "
+        "We should now remove our polyfill and depend on V8's version of these symbols.");
+
+    symbol.set(js, "dispose", js.symbolDispose());
+    symbol.set(js, "asyncDispose", js.symbolAsyncDispose());
+  });
+}
+
+v8::Local<v8::Symbol> getSymbolDispose(v8::Isolate* isolate) {
+  return IsolateBase::from(isolate).getSymbolDispose();
+}
+v8::Local<v8::Symbol> getSymbolAsyncDispose(v8::Isolate* isolate) {
+  return IsolateBase::from(isolate).getSymbolAsyncDispose();
+}
+
 void throwIfConstructorCalledAsFunction(
     const v8::FunctionCallbackInfo<v8::Value>& args,
     const std::type_info& type) {

src/workerd/jsg/resource.h

@@ -566,6 +566,12 @@ constexpr bool hasConstructorMethod(...) { return false; }
 //     };
 void exposeGlobalScopeType(v8::Isolate* isolate, v8::Local<v8::Context> context);
 
+// Polyfill Symbol.dispose and Symbol.asyncDispose.
+void polyfillSymbols(jsg::Lock& js, v8::Local<v8::Context> context);
+
+v8::Local<v8::Symbol> getSymbolDispose(v8::Isolate* isolate);
+v8::Local<v8::Symbol> getSymbolAsyncDispose(v8::Isolate* isolate);
+
 // A configuration type that can be derived from any input type, because it contains nothing.
 class NullConfiguration {
 public:
@@ -926,6 +932,24 @@ struct ResourceTypeBuilder {
         v8::PropertyAttribute::DontEnum);
   }
 
+  template<const char* name, typename Method, Method method>
+  inline void registerDispose() {
+    prototype->Set(getSymbolDispose(isolate), v8::FunctionTemplate::New(isolate,
+        &MethodCallback<TypeWrapper, name, isContext, Self, Method, method,
+                        ArgumentIndexes<Method>>::callback,
+        v8::Local<v8::Value>(), signature, 0, v8::ConstructorBehavior::kThrow),
+        v8::PropertyAttribute::DontEnum);
+  }
+
+  template<const char* name, typename Method, Method method>
+  inline void registerAsyncDispose() {
+    prototype->Set(getSymbolAsyncDispose(isolate), v8::FunctionTemplate::New(isolate,
+        &MethodCallback<TypeWrapper, name, isContext, Self, Method, method,
+                        ArgumentIndexes<Method>>::callback,
+        v8::Local<v8::Value>(), signature, 0, v8::ConstructorBehavior::kThrow),
+        v8::PropertyAttribute::DontEnum);
+  }
+
   template<typename Type, const char* name>
   inline void registerNestedType() {
     static_assert(Type::JSG_KIND == ::workerd::jsg::JsgKind::RESOURCE,
@@ -1225,6 +1249,7 @@ class ResourceWrapper {
     ptr->setModuleRegistry(kj::mv(moduleRegistry));
 
     return JSG_WITHIN_CONTEXT_SCOPE(js, context, [&](jsg::Lock& js) {
+      polyfillSymbols(js, context);
       setupJavascript(js);
       return JsContext<T>(context, kj::mv(ptr));
     });

src/workerd/jsg/rtti.capnp

@@ -212,6 +212,16 @@ struct Structure {
   asyncIterator @7 :Method;
   # Method returning async iterator if the structure is async iterable
 
+  disposable @13 :Bool;
+  # true if the structure is disposable
+  dispose @14 :Method;
+  # dispose method
+
+  asyncDisposable @15 :Bool;
+  # true if the structure is async disposable
+  asyncDispose @16 :Method;
+  # asyncDispose method
+
   tsRoot @8 :Bool;
   # See `JSG_TS_ROOT`'s documentation in the `## TypeScript` section of the JSG README.md.
   # If `JSG_(STRUCT_)TS_ROOT` is declared for a type, this value will be `true`.

src/workerd/jsg/rtti.h

@@ -570,6 +570,12 @@ struct MemberCounter {
   template<const char* name, typename Method, Method method>
   inline void registerAsyncIterable() { /* not a member */ }
 
+  template<const char* name, typename Method, Method method>
+  inline void registerDispose() { /* not a member */ }
+
+  template<const char* name, typename Method, Method method>
+  inline void registerAsyncDispose() { /* not a member */ }
+
   template<typename Type, const char* name>
   inline void registerNestedType() { ++members; }
 
@@ -795,6 +801,30 @@ struct MembersBuilder {
     TupleRttiBuilder<Configuration, Args>::build(method.initArgs(std::tuple_size_v<Args>), rtti);
   }
 
+  template<const char* name, typename Method, Method>
+  inline void registerDispose() {
+    structure.setDisposable(true);
+
+    auto method = structure.initDispose();
+    method.setName(name);
+    using Traits = FunctionTraits<Method>;
+    BuildRtti<Configuration, typename Traits::ReturnType>::build(method.initReturnType(), rtti);
+    using Args = typename Traits::ArgsTuple;
+    TupleRttiBuilder<Configuration, Args>::build(method.initArgs(std::tuple_size_v<Args>), rtti);
+  }
+
+  template<const char* name, typename Method, Method>
+  inline void registerAsyncDispose() {
+    structure.setAsyncDisposable(true);
+
+    auto method = structure.initAsyncDispose();
+    method.setName(name);
+    using Traits = FunctionTraits<Method>;
+    BuildRtti<Configuration, typename Traits::ReturnType>::build(method.initReturnType(), rtti);
+    using Args = typename Traits::ArgsTuple;
+    TupleRttiBuilder<Configuration, Args>::build(method.initArgs(std::tuple_size_v<Args>), rtti);
+  }
+
   inline void registerTypeScriptRoot() {
     structure.setTsRoot(true);
   }

src/workerd/jsg/setup.c++

@@ -360,15 +360,24 @@ IsolateBase::IsolateBase(const V8System& system, v8::Isolate::CreateParams&& cre
 
     ptr->GetHeapProfiler()->AddBuildEmbedderGraphCallback(buildEmbedderGraph, this);
 
-    // Create opaqueTemplate
     {
       // We don't need a v8::Locker here since there's no way another thread could be using the
       // isolate yet, but we do need v8::Isolate::Scope.
       v8::Isolate::Scope isolateScope(ptr);
       v8::HandleScope scope(ptr);
+
+      // Create opaqueTemplate
       auto opaqueTemplate = v8::FunctionTemplate::New(ptr, &throwIllegalConstructor);
       opaqueTemplate->InstanceTemplate()->SetInternalFieldCount(Wrappable::INTERNAL_FIELD_COUNT);
       this->opaqueTemplate.Reset(ptr, opaqueTemplate);
+
+      // Create Symbol.dispose and Symbol.asyncDispose.
+      symbolDispose.Reset(ptr, v8::Symbol::New(ptr,
+          v8::String::NewFromUtf8(ptr, "dispose",
+              v8::NewStringType::kInternalized).ToLocalChecked()));
+      symbolAsyncDispose.Reset(ptr, v8::Symbol::New(ptr,
+          v8::String::NewFromUtf8(ptr, "asyncDispose",
+              v8::NewStringType::kInternalized).ToLocalChecked()));
     }
   });
 }
@@ -395,7 +404,9 @@ void IsolateBase::dropWrappers(kj::Own<void> typeWrapperInstance) {
     // may call into the heap tracer.
     KJ_DEFER(heapTracer.destroy());
 
-    // Make sure opaqueTemplate is destroyed under lock (but not until later).
+    // Make sure v8::Globals are destroyed under lock (but not until later).
+    KJ_DEFER(symbolAsyncDispose.Reset());
+    KJ_DEFER(symbolDispose.Reset());
     KJ_DEFER(opaqueTemplate.Reset());
 
     // Make sure the TypeWrapper is destroyed under lock by declaring a new copy of the variable

src/workerd/jsg/setup.h

@@ -138,6 +138,13 @@ class IsolateBase {
   size_t jsgGetMemorySelfSize() const { return sizeof(IsolateBase); }
   bool jsgGetMemoryInfoIsRootNode() const { return true; }
 
+  JsSymbol getSymbolDispose() {
+    return JsSymbol(symbolDispose.Get(ptr));
+  }
+  JsSymbol getSymbolAsyncDispose() {
+    return JsSymbol(symbolAsyncDispose.Get(ptr));
+  }
+
 private:
   template <typename TypeWrapper>
   friend class Isolate;
@@ -189,6 +196,10 @@ class IsolateBase {
   // object with 2 internal fields.
   v8::Global<v8::FunctionTemplate> opaqueTemplate;
 
+  // Polyfilled Symbol.dispose and Symbol.asyncDispose.
+  v8::Global<v8::Symbol> symbolDispose;
+  v8::Global<v8::Symbol> symbolAsyncDispose;
+
   // We expect queues to remain relatively small -- 8 is the largest size I have observed from local
   // testing.
   static constexpr auto DESTRUCTION_QUEUE_INITIAL_SIZE = 8;