Browse Source

fix(subprocess): preserve runner result boundary

pku-xht 1 month ago
parent
commit
6e1b8eda77

+ 5 - 15
packages/subprocess/subprocess-local/src/linux-scope.ts

@@ -214,7 +214,6 @@ export function launchLinuxScope(
   })
   const closed = observeChildClose(child)
   let forceKillAttempted = false
-  let scopeSettled = false
   const owner = new SystemdScopeOwner(
     `${unitBase}.scope`,
     systemctl,
@@ -223,20 +222,11 @@ export function launchLinuxScope(
     child,
     () => { forceKillAttempted = true },
   )
-  const resultFinalized = closed.then(async () => {
-    try {
-      await owner.waitForExit()
-      scopeSettled = true
-    } catch (_rangeObservationFailed) {
-      // waitForExit retains the authoritative owner failure for its caller.
-    }
-  })
-  const result = runnerDirectResult(child, files, resultFinalized)
-  const direct = result.direct.catch((error: unknown): SubprocessOutcome => {
-    if (forceKillAttempted && scopeSettled && error instanceof DirectResultUnavailableError) {
-      return { exitCode: null, signal: 'SIGKILL' }
-    }
-    throw error
+  const result = runnerDirectResult(child, files, closed)
+  const direct = result.direct.catch(async (error: unknown): Promise<SubprocessOutcome> => {
+    if (!forceKillAttempted || !(error instanceof DirectResultUnavailableError)) throw error
+    await owner.waitForExit()
+    return { exitCode: null, signal: 'SIGKILL' }
   })
   cleanupAfterRunner(files, direct, closed)
   return {

+ 11 - 10
packages/subprocess/subprocess-local/src/runner-launch.ts

@@ -115,22 +115,23 @@ function waitForRunnerHandshake(child: ChildProcess, files: RunnerFiles): Runner
 async function waitForDirectResult(
   files: RunnerFiles,
   initial: RunnerEvent[],
-  resultFinalized: Promise<void>,
+  closed: Promise<void>,
 ): Promise<SubprocessOutcome> {
   let seen = 0
-  const resultState = { finalized: false }
-  void resultFinalized.then(() => { resultState.finalized = true })
+  const wrapperState = { closed: false }
+  void closed.then(() => { wrapperState.closed = true })
   for (;;) {
-    // A read started before finalization may return a stale snapshot. Only a
-    // read started afterward can prove no terminal event remains forthcoming.
-    const finalizedBeforeRead = resultState.finalized
+    // A read started before close may return a stale snapshot after close has
+    // become visible. Only a read started after close can prove no terminal
+    // event was written before the runner exited.
+    const closedBeforeRead = wrapperState.closed
     const events = await readRunnerEventsAsync(files.eventsPath)
     for (const event of events.slice(seen)) {
       if (event.type === 'exit') return { exitCode: event.exitCode, signal: event.signal }
       if (event.type === 'spawn-error' || event.type === 'runner-error') throw deserializeSpawnError(event.error)
     }
     seen = Math.max(seen, events.length, initial.length)
-    if (finalizedBeforeRead) {
+    if (closedBeforeRead) {
       throw new DirectResultUnavailableError('native subprocess runner exited without a direct-command result')
     }
     await sleepMs(RUNNER_EVENT_POLL_MS)
@@ -141,13 +142,13 @@ async function waitForDirectResult(
  * Bind runner events into one direct result while preserving the target pid.
  * @param child - native wrapper process.
  * @param files - private request and result paths.
- * @param resultFinalized - platform proof that no later runner event can arrive.
+ * @param closed - wrapper close observation attached before the start handshake.
  * @returns target pid, direct result, and whether the runner already reported a pre-start terminal failure.
  */
 export function runnerDirectResult(
   child: ChildProcess,
   files: RunnerFiles,
-  resultFinalized: Promise<void>,
+  closed: Promise<void>,
 ): {
   pid: number
   direct: Promise<SubprocessOutcome>
@@ -162,7 +163,7 @@ export function runnerDirectResult(
   }
   return {
     pid: handshake.pid,
-    direct: waitForDirectResult(files, handshake.events, resultFinalized),
+    direct: waitForDirectResult(files, handshake.events, closed),
     failureReported: handshake.failureReported,
   }
 }

+ 1 - 1
packages/subprocess/subprocess-local/tests/linux-scope.spec.ts

@@ -113,7 +113,7 @@ describe.skipIf(process.platform === 'win32')('Linux systemd scope adapter', ()
     expect(systemdArgs).not.toContain('literal $VALUE')
   })
 
-  it('uses a successful scope KILL when the runner cannot publish the direct result', async () => {
+  it('uses a scope KILL after the owner proves the range empty', async () => {
     let wrapper: ReturnType<typeof spawn> | undefined
     const run = vi.fn((_command: string, args: readonly string[], options: Parameters<typeof spawn>[2]) => {
       const separator = args.indexOf('--')

+ 4 - 4
packages/subprocess/subprocess-local/tests/spawn-runner.spec.ts

@@ -333,7 +333,7 @@ describe('spawn runner transport', () => {
 
   })
 
-  it('requires an event snapshot started after result finalization before reporting a missing result', async () => {
+  it('requires an event snapshot started after wrapper close before reporting a missing result', async () => {
     const staleRead = Promise.withResolvers<Awaited<ReturnType<typeof readRunnerEventsAsync>>>()
     let readCount = 0
     vi.resetModules()
@@ -351,12 +351,12 @@ describe('spawn runner transport', () => {
     const files = createRunnerFiles({ argv: ['node'], cwd: '.', env: {} })
     try {
       appendRunnerEvent(files.eventsPath, { type: 'started', pid: 456 })
-      const finalized = Promise.withResolvers<undefined>()
+      const closed = Promise.withResolvers<undefined>()
       const isolated = await import('../src/runner-launch.ts')
-      const result = isolated.runnerDirectResult(fakeChild(123), files, finalized.promise)
+      const result = isolated.runnerDirectResult(fakeChild(123), files, closed.promise)
       expect(result.failureReported).toBe(false)
       expect(readCount).toBe(1)
-      finalized.resolve(undefined)
+      closed.resolve(undefined)
       await Promise.resolve()
       appendRunnerEvent(files.eventsPath, { type: 'exit', exitCode: 0, signal: null })
       staleRead.resolve([{ type: 'started', pid: 456 }])