fix(deno): close all subprocess resources

.github/workflows/ci.yml

@@ -29,7 +29,7 @@ jobs:
         node-version: 14
 
     - name: Setup Deno 1.x
-      uses: denolib/setup-deno@v2
+      uses: denoland/setup-deno@main
       with:
         deno-version: v1.x
 

Makefile

@@ -52,7 +52,7 @@ test-wasm-browser: platform-wasm | scripts/browser/node_modules
 	cd scripts/browser && node browser-tests.js
 
 test-deno: esbuild platform-deno
-	ESBUILD_BINARY_PATH="$(shell pwd)/esbuild" deno run --allow-run --allow-env --allow-net --allow-read --allow-write scripts/deno-tests.js
+	ESBUILD_BINARY_PATH="$(shell pwd)/esbuild" deno test --allow-run --allow-env --allow-net --allow-read --allow-write --no-check scripts/deno-tests.js
 
 register-test: cmd/esbuild/version.go | scripts/node_modules
 	cd npm/esbuild && npm version "$(ESBUILD_VERSION)" --allow-same-version

lib/deno/mod.ts

@@ -177,7 +177,13 @@ let ensureServiceIsRunning = (): Promise<Service> => {
       })
 
       stopService = () => {
+        // Close all resources related to the subprocess.
+        child.stdin.close()
+        child.stdout.close()
         child.close()
+        initializeWasCalled = false;
+        longLivedService = undefined;
+        stopService = undefined
       }
 
       let writeQueue: Uint8Array[] = []
@@ -212,6 +218,13 @@ let ensureServiceIsRunning = (): Promise<Service> => {
           readFromStdout(stdoutBuffer.subarray(0, n))
           readMoreStdout()
         }
+      }).catch(e => {
+        if (e instanceof Deno.errors.Interrupted || e instanceof Deno.errors.BadResource) {
+          // ignore the error if read was interrupted (stdout was closed)
+          afterClose()
+        } else {
+          throw e;
+        }
       })
       readMoreStdout()
 

scripts/deno-tests.js

@@ -1,97 +1,74 @@
 // To run this, you must first build the Deno package with "make platform-deno"
 import * as esbuild from '../deno/mod.js'
-import * as path from 'https://deno.land/std@0.93.0/path/mod.ts'
-import * as asserts from 'https://deno.land/std@0.93.0/testing/asserts.ts'
+import * as path from 'https://deno.land/std@0.95.0/path/mod.ts'
+import * as asserts from 'https://deno.land/std@0.95.0/testing/asserts.ts'
 
 const rootTestDir = path.join(path.dirname(path.fromFileUrl(import.meta.url)), '.deno-tests')
 
-let tests = {
-  async basicBuild({ testDir }) {
-    const input = path.join(testDir, 'in.ts')
-    const dep = path.join(testDir, 'dep.ts')
-    const output = path.join(testDir, 'out.ts')
-    await Deno.writeTextFile(input, 'import dep from "./dep.ts"; export default dep === 123')
-    await Deno.writeTextFile(dep, 'export default 123')
-    await esbuild.build({
-      entryPoints: [input],
-      bundle: true,
-      outfile: output,
-      format: 'esm',
-    })
-    const result = await import(path.toFileUrl(output))
-    asserts.assertStrictEquals(result.default, true)
-  },
-
-  async largeBuild() {
-    // This should be large enough to be bigger than Deno's write buffer
-    let x = '0'
-    for (let i = 0; i < 1000; i++)x += '+' + i
-    x += ','
-    let y = 'return['
-    for (let i = 0; i < 1000; i++)y += x
-    y += ']'
-    const result = await esbuild.build({
-      stdin: {
-        contents: y,
-      },
-      write: false,
-      minify: true,
-    })
-    asserts.assertStrictEquals(result.outputFiles[0].text, y.slice(0, -2) + '];\n')
-  },
-
-  async basicTransform() {
-    const ts = 'let x: number = 1+2'
-    const result = await esbuild.transform(ts, { loader: 'ts' })
-    asserts.assertStrictEquals(result.code, 'let x = 1 + 2;\n')
-  },
+try {
+  Deno.removeSync(rootTestDir, { recursive: true })
+} catch {
+}
+Deno.mkdirSync(rootTestDir, { recursive: true })
 
-  async largeTransform() {
-    // This should be large enough to use the file system passing optimization
-    let x = '0'
-    for (let i = 0; i < 1000; i++)x += '+' + i
-    x += ','
-    let y = 'return['
-    for (let i = 0; i < 1000; i++)y += x
-    y += ']'
-    const result = await esbuild.transform(y, { minify: true })
-    asserts.assertStrictEquals(result.code, y.slice(0, -2) + '];\n')
-  },
+function test(name, fn) {
+  let testDir = path.join(rootTestDir, name)
+  Deno.test(name, async () => {
+    await Deno.mkdir(testDir, { recursive: true })
+    try {
+      await fn({ testDir })
+    } finally {
+      await Deno.remove(testDir, { recursive: true }).catch(() => null)
+      esbuild.stop()
+    }
+  })
 }
 
-async function main() {
+window.addEventListener("unload", () => {
   try {
     Deno.removeSync(rootTestDir, { recursive: true })
   } catch {
+    // root test dir possibly already removed, so ignore
   }
-  Deno.mkdirSync(rootTestDir, { recursive: true })
+}) 
 
-  // Run all tests concurrently
-  const runTest = async ([name, fn]) => {
-    let testDir = path.join(rootTestDir, name)
-    try {
-      await Deno.mkdir(testDir, { recursive: true })
-      await fn({ testDir })
-      await Deno.remove(testDir, { recursive: true }).catch(() => null)
-      return true
-    } catch (e) {
-      console.error(`❌ ${name}: ${e && e.stack || e}`)
-      return false
-    }
-  }
-  const allTestsPassed = (await Promise.all(Object.entries(tests).map(runTest))).every(success => success)
-  esbuild.stop()
+test("basicBuild", async ({ testDir }) => {
+  const input = path.join(testDir, 'in.ts')
+  const dep = path.join(testDir, 'dep.ts')
+  const output = path.join(testDir, 'out.ts')
+  await Deno.writeTextFile(input, 'import dep from "./dep.ts"; export default dep === 123')
+  await Deno.writeTextFile(dep, 'export default 123')
+  await esbuild.build({
+    entryPoints: [input],
+    bundle: true,
+    outfile: output,
+    format: 'esm',
+  })
+  const result = await import(path.toFileUrl(output))
+  asserts.assertStrictEquals(result.default, true)
+})
 
-  if (!allTestsPassed) {
-    console.error(`❌ deno tests failed`)
-    Deno.exit(1)
-  } else {
-    console.log(`✅ deno tests passed`)
-    try {
-      Deno.removeSync(rootTestDir, { recursive: true })
-    } catch {
-    }
-  }
-}
+test("basicTransform", async () => {
+  const ts = 'let x: number = 1+2'
+  const result = await esbuild.transform(ts, { loader: 'ts' })
+  asserts.assertStrictEquals(result.code, 'let x = 1 + 2;\n')
+})
+
+test("largeTransform", async () => {
+  // This should be large enough to be bigger than Deno's write buffer
+  let x = '0'
+  for (let i = 0; i < 1000; i++)x += '+' + i
+  x += ','
+  let y = 'return['
+  for (let i = 0; i < 1000; i++)y += x
+  y += ']'
+  const result = await esbuild.build({
+    stdin: {
+      contents: y,
+    },
+    write: false,
+    minify: true,
+  })
+  asserts.assertStrictEquals(result.outputFiles[0].text, y.slice(0, -2) + '];\n')
+})
 
-await main()