From cf92e69f739289060b0b6ede50937dfbc0a2704d Mon Sep 17 00:00:00 2001 From: Victor Adossi Date: Wed, 29 Jul 2026 19:22:46 +0900 Subject: [PATCH 1/3] fix(bindgen): dispose component resources correctly --- .../src/function_bindgen.rs | 39 ++++++++++--------- crates/js-component-bindgen/src/ts_bindgen.rs | 8 ++-- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/crates/js-component-bindgen/src/function_bindgen.rs b/crates/js-component-bindgen/src/function_bindgen.rs index 2c4cc87c2..5e8fc2aae 100644 --- a/crates/js-component-bindgen/src/function_bindgen.rs +++ b/crates/js-component-bindgen/src/function_bindgen.rs @@ -2300,25 +2300,26 @@ impl Bindgen for FunctionBindgen<'_> { uwriteln!(self.src, "Object.defineProperty({rsc}, {symbol_resource_handle}, {{ writable: true, value: {handle} }}); finalizationRegistry{tid}.register({rsc}, {handle}, {rsc});"); - if let Some(dtor) = dtor_name { - // The Symbol.dispose function gets disabled on drop, so we can rely on the own handle remaining valid. - uwriteln!( - self.src, - "Object.defineProperty({rsc}, {symbol_dispose}, {{ writable: true, value: function () {{ - finalizationRegistry{tid}.unregister({rsc}); - {rsc_table_remove}(handleTable{tid}, {handle}); - {rsc}[{symbol_dispose}] = {empty_func}; - {rsc}[{symbol_resource_handle}] = undefined; - {dtor}(handleTable{tid}[({handle} << 1) + 1] & ~{rsc_flag}); - }}}});" - ); - } else { - // Set up Symbol.dispose for borrows to allow its call, even though it does nothing. - uwriteln!( - self.src, - "Object.defineProperty({rsc}, {symbol_dispose}, {{ writable: true, value: {empty_func} }});", - ); - } + let dtor_call = dtor_name + .as_ref() + .map(|dtor| format!("{dtor}(handleEntry.rep);")) + .unwrap_or_default(); + // Explicitly dropping an own handle must always release the host-side + // handle and finalizer registration. A component-defined destructor is + // an additional callback, not a prerequisite for resource cleanup. + // + // Disable Symbol.dispose and clear the handle before calling the + // component destructor so repeated or re-entrant disposal is a no-op. + uwriteln!( + self.src, + "Object.defineProperty({rsc}, {symbol_dispose}, {{ writable: true, value: function () {{ + finalizationRegistry{tid}.unregister({rsc}); + const handleEntry = {rsc_table_remove}(handleTable{tid}, {handle}); + {rsc}[{symbol_dispose}] = {empty_func}; + {rsc}[{symbol_resource_handle}] = undefined; + {dtor_call} + }}}});" + ); } else { // Borrow handles of local resources have rep handles, which we carry through here. uwriteln!( diff --git a/crates/js-component-bindgen/src/ts_bindgen.rs b/crates/js-component-bindgen/src/ts_bindgen.rs index 4ed69fb03..e4f1e736b 100644 --- a/crates/js-component-bindgen/src/ts_bindgen.rs +++ b/crates/js-component-bindgen/src/ts_bindgen.rs @@ -876,9 +876,11 @@ impl<'a> TsInterface<'a> { fn finish(mut self) -> (Source, BTreeSet) { for (resource, (meta, source)) in self.resources { - let (impl_phrase, extra_members) = if self.is_guest && meta.is_import() { - // Resources types imported in the guest will have generated [Symbol.dispose]() - // for resource cleanup, but this is not the case for any other scenario. + let generates_disposable_resource = + (self.is_guest && meta.is_import()) || (!self.is_guest && meta.is_export()); + let (impl_phrase, extra_members) = if generates_disposable_resource { + // Guest imports and host exports are resource consumers. Their generated + // wrappers own component-model handles and support deterministic cleanup. (" implements Disposable", vec!["[Symbol.dispose](): void;"]) } else { ("", vec![]) From d809faed1f8d06d715fb0a3351cb506dd1aef626 Mon Sep 17 00:00:00 2001 From: Victor Adossi Date: Wed, 29 Jul 2026 19:23:24 +0900 Subject: [PATCH 2/3] test(transpile): add a test for component resource disposal --- .../disposable-resources.wit | 11 ++- .../jco-transpile/test/p3/future-lifts.ts | 11 +++ packages/jco-transpile/test/resources.ts | 68 ++++++++++++++++++- packages/jco-transpile/test/typescript.ts | 30 ++++++-- 4 files changed, 111 insertions(+), 9 deletions(-) diff --git a/packages/jco-transpile/test/fixtures/wit/disposable-resources/disposable-resources.wit b/packages/jco-transpile/test/fixtures/wit/disposable-resources/disposable-resources.wit index 8ca60656f..f8f1c0c1e 100644 --- a/packages/jco-transpile/test/fixtures/wit/disposable-resources/disposable-resources.wit +++ b/packages/jco-transpile/test/fixtures/wit/disposable-resources/disposable-resources.wit @@ -18,8 +18,17 @@ interface resources { create-connection: func(url: string) -> connection; } +interface exported-resources { + /// A component-defined resource consumed by the host. + resource transaction { + constructor(); + commit: func(); + } +} + world disposable-world { import resources; + export exported-resources; /// Also test a world-level resource resource database { @@ -28,4 +37,4 @@ world disposable-world { } export run: func() -> string; -} \ No newline at end of file +} diff --git a/packages/jco-transpile/test/p3/future-lifts.ts b/packages/jco-transpile/test/p3/future-lifts.ts index 810da897f..112e0587b 100644 --- a/packages/jco-transpile/test/p3/future-lifts.ts +++ b/packages/jco-transpile/test/p3/future-lifts.ts @@ -436,6 +436,17 @@ suite('future lifts', () => { instance['jco:test-components/get-future-async'].getExampleResourceOwnDisposes(), numDisposed, ); + assert.doesNotThrow(() => resource[disposeSymbol](), 'resource disposal should be idempotent'); + assert.strictEqual( + instance['jco:test-components/get-future-async'].getExampleResourceOwnDisposes(), + numDisposed, + 'disposing the same handle twice must not run the guest destructor twice', + ); + assert.throws( + () => resource.getId(), + /not a valid .* resource/i, + 'a disposed resource wrapper must no longer be usable', + ); } }); diff --git a/packages/jco-transpile/test/resources.ts b/packages/jco-transpile/test/resources.ts index 1044703e6..607d79fab 100644 --- a/packages/jco-transpile/test/resources.ts +++ b/packages/jco-transpile/test/resources.ts @@ -1,9 +1,13 @@ +import { readFile } from 'node:fs/promises'; import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; -import { suite, test } from 'vitest'; +import { assert, suite, test } from 'vitest'; import { WASIShim } from '@bytecodealliance/preview2-shim/instantiation'; +import { transpileBytes } from '../src/index.js'; +import { componentEmbed, componentNew } from '../src/wasm-tools.js'; import { setupAsyncTest } from './helpers.js'; import { LOCAL_TEST_COMPONENTS_DIR } from './common.js'; @@ -11,6 +15,9 @@ suite('resources', () => { // Ensure trampoline code is called on external object with relevant `this`. // see: https://github.com/bytecodealliance/jco/issues/1313 test('simple imported resource call', async () => { + const disposeSymbol = Symbol.dispose || Symbol.for('dispose'); + let disposed = 0; + class IncrementingExampleResource { private id: number; @@ -22,6 +29,10 @@ suite('resources', () => { this.id += 1; return this.id; } + + [disposeSymbol]() { + disposed += 1; + } } const name = 'resource-incrementing-id'; @@ -43,7 +54,62 @@ suite('resources', () => { }); instance['jco:test-components/local-run'].run(); + assert.strictEqual(disposed, 1, 'dropping the guest-owned import should dispose the host resource once'); await cleanup(); }); + + test('component-defined resource disposal uses the removed handle representation', async () => { + const fixture = fileURLToPath( + new URL('./fixtures/components/runtime/resources.2.component.wat', import.meta.url), + ); + const { files } = await transpileBytes(await readFile(fixture), { + name: 'resource-disposal', + minify: false, + }); + const source = new TextDecoder().decode(files['resource-disposal.js']); + + assert.match( + source, + /const handleEntry = rscTableRemove\(handleTable\d+, handle\d+\);[\s\S]*?\['0'\]\(handleEntry\.rep\);/, + 'explicit disposal should call the destructor with the representation returned while removing the handle', + ); + assert.notMatch( + source, + /rscTableRemove\(handleTable\d+, handle\d+\);[\s\S]{0,300}?handleTable\d+\[\(handle\d+ << 1\) \+ 1\]/, + 'explicit disposal must not read a representation from a removed handle-table entry', + ); + }); + + test('component-defined resource without a destructor still releases its handle', async () => { + const component = await componentNew( + await componentEmbed({ + dummy: true, + witSource: ` + package test:resource-disposal; + + interface resources { + resource no-destructor { + constructor(); + } + } + + world test { + export resources; + } + `, + }), + ); + const { files } = await transpileBytes(component, { + name: 'resource-without-destructor', + minify: false, + }); + const source = new TextDecoder().decode(files['resource-without-destructor.js']); + + assert.match( + source, + /Object\.defineProperty\(rsc\d+, symbolDispose,[\s\S]*?const handleEntry = rscTableRemove\(handleTable\d+, handle\d+\);[\s\S]*?symbolRscHandle\] = undefined;/, + 'explicit disposal should release and invalidate an owned handle even without a guest destructor', + ); + }); }); diff --git a/packages/jco-transpile/test/typescript.ts b/packages/jco-transpile/test/typescript.ts index ed8b51699..8d53cad27 100644 --- a/packages/jco-transpile/test/typescript.ts +++ b/packages/jco-transpile/test/typescript.ts @@ -47,11 +47,7 @@ suite(`TypeScript`, async () => { assert.ok(dtsSource.includes(`export function foobarbaz(): Array;`)); }); - // NOTE: somewhat confusingly, host generation of resources should *not* - // generation Disposable, because the embedder may or may not choose to make - // the implemented resource import disposable or not. - // - test.concurrent(`Disposable interface generation (host-types, import) `, async () => { + test.concurrent(`Disposable interface generation (host types)`, async () => { const witSource = await readFile( fileURLToPath(new URL(`./fixtures/wit/disposable-resources/disposable-resources.wit`, import.meta.url)), 'utf8', @@ -70,7 +66,7 @@ suite(`TypeScript`, async () => { [`export class Database implements Disposable {`, `[Symbol.dispose](): void;`].every( (s) => !mainDtsSource.includes(s), ), - 'Database resource should not implement Disposable interface', + 'host-provided world-level Database resource should not require Disposable', ); const interfaceDtsSource = new TextDecoder().decode( @@ -82,7 +78,17 @@ suite(`TypeScript`, async () => { `export class Connection implements Disposable {`, `[Symbol.dispose](): void;`, ].every((s) => !interfaceDtsSource.includes(s)), - 'FileHandle/Connection resources should not implement Disposable interface', + 'host-provided FileHandle/Connection resource imports should not require Disposable', + ); + + const exportedInterfaceDtsSource = new TextDecoder().decode( + files['interfaces/test-disposable-resources-exported-resources.d.ts'], + ); + assert( + [`export class Transaction implements Disposable {`, `[Symbol.dispose](): void;`].every((s) => + exportedInterfaceDtsSource.includes(s), + ), + 'component-exported Transaction resource should implement Disposable', ); }); @@ -113,6 +119,16 @@ suite(`TypeScript`, async () => { ].every((s) => interfaceDtsSource.includes(s)), 'FileHandle/Connection resources should implement Disposable interface', ); + + const exportedInterfaceDtsSource = new TextDecoder().decode( + files['interfaces/test-disposable-resources-exported-resources.d.ts'], + ); + assert( + [`export class Transaction implements Disposable {`, `[Symbol.dispose](): void;`].every( + (s) => !exportedInterfaceDtsSource.includes(s), + ), + 'guest-provided Transaction implementation should not require Disposable', + ); }); // Regression test for https://github.com/bytecodealliance/jco/issues/1708 From 2239af421d08d23cea852ec4542d8d1b28df3ae8 Mon Sep 17 00:00:00 2001 From: Victor Adossi Date: Wed, 29 Jul 2026 19:24:09 +0900 Subject: [PATCH 3/3] test(jco): add test for CM destructor metadata fix --- packages/jco/test/componentize.js | 32 +++++++++++++++++++ .../componentize/resource-disposal/source.js | 22 +++++++++++++ .../componentize/resource-disposal/source.wit | 14 ++++++++ 3 files changed, 68 insertions(+) create mode 100644 packages/jco/test/fixtures/componentize/resource-disposal/source.js create mode 100644 packages/jco/test/fixtures/componentize/resource-disposal/source.wit diff --git a/packages/jco/test/componentize.js b/packages/jco/test/componentize.js index 7a0af7cdb..da92fb91c 100644 --- a/packages/jco/test/componentize.js +++ b/packages/jco/test/componentize.js @@ -15,6 +15,38 @@ import { exec, getTmpDir, jcoPath } from "./helpers.js"; // used widely, we can switch to regular dynamic imports, as componentize-js // versions are real dependencies now. suite("componentize", () => { + // ComponentizeJS currently omits the component-model destructor metadata for + // a JS-exported resource. + // + // TODO: flip this test once componentize-js is fixed + // see: https://github.com/bytecodealliance/jco/issues/989#issuecomment-4550236825 + test.fails("componentized exported resource invokes its guest destructor", async () => { + const fixtureDir = join(COMPONENT_JS_FIXTURES_DIR, "resource-disposal"); + const outputDir = await getTmpDir(); + const componentPath = join(outputDir, "component.wasm"); + const transpiledDir = join(outputDir, "transpiled"); + + await exec( + jcoPath, + "componentize", + join(fixtureDir, "source.js"), + "--disable", + "all", + "-w", + join(fixtureDir, "source.wit"), + "-o", + componentPath, + ); + await exec(jcoPath, "transpile", componentPath, "-o", transpiledDir, "--name", "resource-disposal"); + await writeFile(join(transpiledDir, "package.json"), JSON.stringify({ type: "module" })); + + const { resources } = await import(`${pathToFileURL(transpiledDir)}/resource-disposal.js`); + const resource = new resources.Example(42); + assert.strictEqual(resource.getId(), 42); + resource[Symbol.dispose || Symbol.for("dispose")](); + assert.strictEqual(resources.disposeCount(), 1); + }); + test("rejects a custom StarlingMonkey engine with the QuickJS backend", async () => { const fixtureDir = join(COMPONENT_JS_FIXTURES_DIR, "typescript-direct"); const outputDir = await getTmpDir(); diff --git a/packages/jco/test/fixtures/componentize/resource-disposal/source.js b/packages/jco/test/fixtures/componentize/resource-disposal/source.js new file mode 100644 index 000000000..8e8c26145 --- /dev/null +++ b/packages/jco/test/fixtures/componentize/resource-disposal/source.js @@ -0,0 +1,22 @@ +let disposeCount = 0; + +class Example { + constructor(id) { + this.id = id; + } + + getId() { + return this.id; + } + + [Symbol.dispose]() { + disposeCount += 1; + } +} + +export const resources = { + Example, + disposeCount() { + return disposeCount; + }, +}; diff --git a/packages/jco/test/fixtures/componentize/resource-disposal/source.wit b/packages/jco/test/fixtures/componentize/resource-disposal/source.wit new file mode 100644 index 000000000..6e2adc3a5 --- /dev/null +++ b/packages/jco/test/fixtures/componentize/resource-disposal/source.wit @@ -0,0 +1,14 @@ +package local:resource-disposal; + +interface resources { + resource example { + constructor(id: u32); + get-id: func() -> u32; + } + + dispose-count: func() -> u32; +} + +world test { + export resources; +}