fix: bound concurrent devcontainer filesystem processes

.github/workflows/ci.yml

@@ -68,6 +68,9 @@ jobs:
         timeout-minutes: 30
         run: npm run e2e:devcontainer
         if: matrix.os == 'ubuntu-24.04'
+      - name: Container filesystem process lifecycle regression
+        run: node --test packages/devcontainer-node/test/ContainerFileSystem.e2e.ts
+        if: matrix.os == 'ubuntu-24.04'
       - name: Devcontainer direct test URLs and reloads
         timeout-minutes: 15
         run: npm run e2e:local-url --prefix packages/e2e

.github/workflows/pr.yml

@@ -69,6 +69,9 @@ jobs:
         timeout-minutes: 30
         run: npm run e2e:devcontainer
         if: matrix.os == 'ubuntu-24.04'
+      - name: Container filesystem process lifecycle regression
+        run: node --test packages/devcontainer-node/test/ContainerFileSystem.e2e.ts
+        if: matrix.os == 'ubuntu-24.04'
       - name: Devcontainer direct test URLs and reloads
         timeout-minutes: 15
         run: npm run e2e:local-url --prefix packages/e2e

packages/devcontainer-node/src/parts/ContainerFileSystem/ContainerFileSystem.ts

@@ -27,7 +27,10 @@ done`,
   writeFile: 'cat > "$1"',
 }
 
-export const run = async (options: Options): Promise<unknown> => {
+const runOperation = async (
+  options: Options,
+  runProcess: typeof RunProcess.runProcess,
+): Promise<unknown> => {
   const {
     containerCli = 'docker',
     containerId,
@@ -42,7 +45,7 @@ export const run = async (options: Options): Promise<unknown> => {
   if (!script) {
     throw new Error(`Unsupported container file operation: ${operation}`)
   }
-  const result = await RunProcess.runProcess({
+  const result = await runProcess({
     args: [
       'exec',
       '-i',
@@ -81,3 +84,37 @@ export const run = async (options: Options): Promise<unknown> => {
   }
   return result.stdout
 }
+
+// File searches and language services can request hundreds of files at once.
+// Bound Docker exec processes across containers while allowing terminal and
+// lifecycle commands to proceed independently.
+const maxProcesses = 8
+
+export const createContainerFileSystem = (
+  runProcess: typeof RunProcess.runProcess,
+) => {
+  let activeProcesses = 0
+  const waiting: (() => void)[] = []
+  return async (options: Options): Promise<unknown> => {
+    if (activeProcesses < maxProcesses) {
+      activeProcesses++
+    } else {
+      await new Promise<void>((resolve) => {
+        waiting.push(resolve)
+      })
+    }
+    try {
+      return await runOperation(options, runProcess)
+    } finally {
+      const next = waiting.shift()
+      if (next) {
+        // Transfer the slot directly so a new request cannot overtake the queue.
+        next()
+      } else {
+        activeProcesses--
+      }
+    }
+  }
+}
+
+export const run = createContainerFileSystem(RunProcess.runProcess)

packages/devcontainer-node/test/ContainerFileSystem.e2e.ts

@@ -0,0 +1,80 @@
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import { test } from 'node:test'
+import * as ContainerFileSystem from '../src/parts/ContainerFileSystem/ContainerFileSystem.ts'
+import * as RunProcess from '../src/parts/RunProcess/RunProcess.ts'
+
+const children = (): string[] => {
+  return readFileSync(`/proc/self/task/${process.pid}/children`, 'utf8')
+    .trim()
+    .split(/\s+/)
+    .filter(Boolean)
+}
+
+void test(
+  'concurrent container file operations bound and reap Docker children',
+  { timeout: 120_000 },
+  async () => {
+    const started = await RunProcess.runProcess({
+      args: [
+        'run',
+        '-d',
+        '--entrypoint',
+        'sleep',
+        'mcr.microsoft.com/devcontainers/base:ubuntu24.04',
+        'infinity',
+      ],
+      command: 'docker',
+    })
+    assert.ok(!('errorMessage' in started), JSON.stringify(started))
+    assert.equal(started.exitCode, 0, started.stderr)
+    const containerId = started.stdout.trim()
+    let peak = 0
+    const sample = () => {
+      peak = Math.max(peak, children().length)
+    }
+    const timer = setInterval(sample, 5)
+    try {
+      for (let cycle = 0; cycle < 3; cycle++) {
+        const results = await Promise.allSettled(
+          Array.from({ length: 128 }, (_, index) =>
+            ContainerFileSystem.run({
+              containerId,
+              operation: 'readFile',
+              path: index % 16 === 0 ? '/missing-file' : '/etc/os-release',
+              remoteWorkspaceFolder: '/',
+            }),
+          ),
+        )
+        for (const [index, result] of results.entries()) {
+          if (index % 16 === 0) {
+            assert.equal(result.status, 'rejected')
+            assert.equal(result.reason.code, 'ENOENT')
+          } else {
+            assert.equal(result.status, 'fulfilled')
+            assert.match(
+              Buffer.from(String(result.value), 'base64').toString(),
+              /Ubuntu/,
+            )
+          }
+        }
+        assert.deepEqual(
+          children(),
+          [],
+          'completed operations must leave no Docker processes or zombies',
+        )
+      }
+      assert.ok(peak > 0, 'must observe the real Docker processes')
+      assert.ok(
+        peak <= 8,
+        `expected at most eight Docker children, observed ${peak}`,
+      )
+    } finally {
+      clearInterval(timer)
+      await RunProcess.runProcess({
+        args: ['rm', '-f', containerId],
+        command: 'docker',
+      })
+    }
+  },
+)

packages/devcontainer-node/test/ContainerFileSystem.test.ts

@@ -0,0 +1,72 @@
+import { expect, jest, test } from '@jest/globals'
+import type {
+  RunProcessOptions,
+  RunProcessResult,
+} from '../src/parts/RunProcess/RunProcess.ts'
+import { createContainerFileSystem } from '../src/parts/ContainerFileSystem/ContainerFileSystem.ts'
+
+const options = {
+  containerId: 'container',
+  operation: 'readFile',
+  path: '/workspace/file',
+  remoteWorkspaceFolder: '/workspace',
+}
+const success = { exitCode: 0, stderr: '', stdout: 'content' }
+
+test('bounds simultaneous container file processes and drains queued reads', async () => {
+  const runProcess =
+    jest.fn<(options: RunProcessOptions) => Promise<RunProcessResult>>()
+  const run = createContainerFileSystem(runProcess)
+  const started: string[] = []
+  const pending: ((result: RunProcessResult) => void)[] = []
+  let active = 0
+  let peak = 0
+  runProcess.mockImplementation(async ({ args }) => {
+    started.push(args.at(-1)!)
+    active++
+    peak = Math.max(peak, active)
+    const result = await new Promise<RunProcessResult>((resolve) => {
+      pending.push(resolve)
+    })
+    active--
+    return result
+  })
+  const paths = Array.from({ length: 32 }, (_, i) => `/workspace/file${i}`)
+  const reads = paths.map((path) => run({ ...options, path }))
+  try {
+    // Each completion admits the next waiting file operation.
+    for (let i = 0; i < paths.length; i++) {
+      await new Promise<void>((resolve) => setImmediate(resolve))
+      pending.shift()!(success)
+    }
+    expect(await Promise.all(reads)).toEqual(paths.map(() => 'content'))
+    expect(started).toEqual(paths)
+    expect(peak).toBeLessThanOrEqual(8)
+  } finally {
+    runProcess.mockReset()
+  }
+})
+
+test('failed operations release their capacity for subsequent reads', async () => {
+  const runProcess =
+    jest.fn<(options: RunProcessOptions) => Promise<RunProcessResult>>()
+  const run = createContainerFileSystem(runProcess)
+  runProcess.mockImplementation(async () => ({
+    exitCode: 44,
+    stderr: '',
+    stdout: '',
+  }))
+  const results = await Promise.allSettled(
+    Array.from({ length: 24 }, () => run(options)),
+  )
+  expect(results).toHaveLength(24)
+  for (const result of results) {
+    expect(result).toMatchObject({
+      reason: { code: 'ENOENT' },
+      status: 'rejected',
+    })
+  }
+  runProcess.mockResolvedValue(success)
+  expect(await run(options)).toBe('content')
+  runProcess.mockReset()
+})