fix(e2b): close adapter lifecycle review gaps
This commit is contained in:
@@ -19,3 +19,23 @@ export function scrubRemoteEnvironment(raw: string): Map<string, string> {
|
||||
}
|
||||
return environment
|
||||
}
|
||||
|
||||
/**
|
||||
* Overlay explicit entries and serialize one validated E2B environment.
|
||||
* @param raw - The complete NUL-delimited remote environment.
|
||||
* @param explicit - Deliberate caller overrides applied after ambient scrubbing.
|
||||
* @returns NUL-delimited `name=value` entries accepted by `env -i`.
|
||||
*/
|
||||
export function serializeRemoteEnvironment(
|
||||
raw: string,
|
||||
explicit: Readonly<Record<string, string>> | undefined,
|
||||
): string {
|
||||
const environment = scrubRemoteEnvironment(raw)
|
||||
for (const [name, value] of Object.entries(explicit ?? {})) {
|
||||
if (name.length === 0 || name.includes('=') || name.includes('\0') || value.includes('\0')) {
|
||||
throw new Error('subprocess-e2b: environment entries require non-empty NUL-free names without = and NUL-free values')
|
||||
}
|
||||
environment.set(name, value)
|
||||
}
|
||||
return [...environment].map(([name, value]) => `${name}=${value}\0`).join('')
|
||||
}
|
||||
|
||||
@@ -28,7 +28,8 @@ export class E2BSubprocessService extends SubprocessService {
|
||||
|
||||
private readonly live = new Set<E2BSubprocessHandle>()
|
||||
private readonly terminals = new Set<SubprocessTerminalHandle>()
|
||||
private readonly terminalSetups = new Set<Promise<void>>()
|
||||
private readonly terminalSetups = new Map<Promise<void>, AbortController>()
|
||||
private readonly failedTerminalSetupCleanups = new Set<() => Promise<void>>()
|
||||
private disposing = false
|
||||
|
||||
/** @inheritdoc */
|
||||
@@ -44,9 +45,13 @@ export class E2BSubprocessService extends SubprocessService {
|
||||
this.runtimeRoot = ctx.e2b.runtimeRoot
|
||||
ctx.effect(() => async () => {
|
||||
this.disposing = true
|
||||
await Promise.all([...this.terminalSetups])
|
||||
for (const controller of this.terminalSetups.values()) {
|
||||
controller.abort(new Error('subprocess-e2b: service disposed during terminal setup'))
|
||||
}
|
||||
await Promise.all([...this.terminalSetups.keys()])
|
||||
const handles = [...this.live]
|
||||
const terminals = [...this.terminals]
|
||||
const failedTerminalSetupCleanups = [...this.failedTerminalSetupCleanups]
|
||||
const pending: Promise<unknown>[] = []
|
||||
for (const handle of handles) {
|
||||
handle.terminate()
|
||||
@@ -59,6 +64,9 @@ export class E2BSubprocessService extends SubprocessService {
|
||||
terminal.terminate()
|
||||
pending.push(terminal.waitForExit().then(() => { this.terminals.delete(terminal) }))
|
||||
}
|
||||
for (const cleanup of failedTerminalSetupCleanups) {
|
||||
pending.push(cleanup().then(() => { this.failedTerminalSetupCleanups.delete(cleanup) }))
|
||||
}
|
||||
await Promise.all(pending)
|
||||
}, 'e2b subprocess teardown')
|
||||
}
|
||||
@@ -133,9 +141,18 @@ export class E2BSubprocessService extends SubprocessService {
|
||||
spec.signal?.throwIfAborted()
|
||||
const stateDir = posix.join(this.runtimeRoot, 'terminals', randomUUID())
|
||||
const setup = Promise.withResolvers<void>()
|
||||
this.terminalSetups.add(setup.promise)
|
||||
const setupController = new AbortController()
|
||||
const setupSignal = spec.signal === undefined
|
||||
? setupController.signal
|
||||
: AbortSignal.any([spec.signal, setupController.signal])
|
||||
this.terminalSetups.set(setup.promise, setupController)
|
||||
try {
|
||||
const terminal = await spawnE2BTerminal(this.ctx.e2b, spec, stateDir)
|
||||
const terminal = await spawnE2BTerminal(
|
||||
this.ctx.e2b,
|
||||
{ ...spec, signal: setupSignal },
|
||||
stateDir,
|
||||
(cleanup) => { this.failedTerminalSetupCleanups.add(cleanup) },
|
||||
)
|
||||
this.terminals.add(terminal)
|
||||
if (this.isDisposing()) {
|
||||
terminal.terminate()
|
||||
|
||||
@@ -17,7 +17,7 @@ import type {
|
||||
SubprocessSpawnSpec,
|
||||
} from '@deepseek-ai/dsh-subprocess'
|
||||
import type E2BSandboxService from '@deepseek-ai/dsh-e2b'
|
||||
import { scrubRemoteEnvironment } from './environment.ts'
|
||||
import { serializeRemoteEnvironment } from './environment.ts'
|
||||
import { E2BBase64Decoder, E2B_OUTPUT_COMPLETE_FRAME, E2BOutputReader } from './output.ts'
|
||||
|
||||
const GROUP_POLL_MS = 20
|
||||
@@ -92,12 +92,6 @@ function withinMs<T>(promise: Promise<T>, timeoutMs: number): Promise<T | undefi
|
||||
})
|
||||
}
|
||||
|
||||
function remoteEnvironment(raw: string, explicit: Readonly<Record<string, string>> | undefined): string {
|
||||
const environment = scrubRemoteEnvironment(raw)
|
||||
for (const [name, value] of Object.entries(explicit ?? {})) environment.set(name, value)
|
||||
return [...environment].map(([name, value]) => `${name}=${value}\0`).join('')
|
||||
}
|
||||
|
||||
function commandText(spec: SubprocessSpawnSpec, paths: RemotePaths): string {
|
||||
const encoder = `"$dsh_e2b_env_bin" -i "$dsh_e2b_node" -e ${quoteE2BShellArg(OUTPUT_ENCODER_SOURCE)}`
|
||||
const stdoutRedirect = hasSpill(spec.stdio.stdout)
|
||||
@@ -213,7 +207,8 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
private invalidHandleQuiescent = false
|
||||
private provisionalHandleQuiescent = false
|
||||
private terminationStarted = false
|
||||
private terminationSucceeded = false
|
||||
private terminationFenced = false
|
||||
private quiescenceProven = false
|
||||
private terminationAttempt: Promise<void> | undefined
|
||||
private terminationFailure: Error | undefined
|
||||
private terminationSignal: NodeJS.Signals | null = null
|
||||
@@ -265,18 +260,18 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
|
||||
/** @inheritdoc */
|
||||
terminate(): void {
|
||||
if (this.terminationSucceeded || this.terminationAttempt !== undefined) return
|
||||
if (this.terminationFenced || this.quiescenceProven || this.terminationAttempt !== undefined) return
|
||||
this.terminationStarted = true
|
||||
this.terminationFailure = undefined
|
||||
const attempt = this.terminateRemote()
|
||||
this.terminationAttempt = attempt
|
||||
void attempt.then(
|
||||
() => {
|
||||
this.terminationSucceeded = true
|
||||
this.terminationFenced = true
|
||||
this.terminationAttempt = undefined
|
||||
},
|
||||
(error: unknown) => {
|
||||
this.terminationFailure = asError(error)
|
||||
if (!this.quiescenceProven) this.terminationFailure = asError(error)
|
||||
this.terminationAttempt = undefined
|
||||
},
|
||||
)
|
||||
@@ -284,17 +279,24 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
|
||||
/** @inheritdoc */
|
||||
async waitForExit(signal?: AbortSignal): Promise<boolean> {
|
||||
if (this.quiescenceProven) return true
|
||||
let handle: CommandHandle | undefined
|
||||
if (this.terminationStarted) {
|
||||
const observed = await waitWithSignal(this.commandState.promise, signal)
|
||||
if (observed === WAIT_ABORTED) return false
|
||||
handle = observed
|
||||
if (handle === undefined) return true
|
||||
if (handle === undefined) {
|
||||
this.markQuiescent()
|
||||
return true
|
||||
}
|
||||
if (this.remotePid <= 0) {
|
||||
const attempt = this.terminationAttempt
|
||||
if (attempt !== undefined && await waitWithSignal(attempt, signal) === WAIT_ABORTED) return false
|
||||
this.throwTerminationFailure()
|
||||
if (this.invalidHandleQuiescent || this.provisionalHandleQuiescent) return true
|
||||
if (this.invalidHandleQuiescent || this.provisionalHandleQuiescent) {
|
||||
this.markQuiescent()
|
||||
return true
|
||||
}
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
@@ -303,7 +305,10 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
handle = observed
|
||||
} catch {
|
||||
handle = this.commandHandle
|
||||
if (handle === undefined) return true
|
||||
if (handle === undefined) {
|
||||
this.markQuiescent()
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
this.throwTerminationFailure()
|
||||
@@ -320,11 +325,18 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
if (!await waitTick(signal)) return false
|
||||
}
|
||||
this.throwTerminationFailure()
|
||||
return !isAborted(signal)
|
||||
if (isAborted(signal)) return false
|
||||
this.markQuiescent()
|
||||
return true
|
||||
}
|
||||
|
||||
private readonly onAbort = (): void => { this.terminate() }
|
||||
|
||||
private markQuiescent(): void {
|
||||
this.quiescenceProven = true
|
||||
this.terminationFailure = undefined
|
||||
}
|
||||
|
||||
private async run(): Promise<SubprocessOutcome> {
|
||||
let sandbox: Sandbox | undefined
|
||||
try {
|
||||
@@ -385,7 +397,7 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
return outcome
|
||||
} catch (error: unknown) {
|
||||
this.commandState.resolve(undefined)
|
||||
let failure = error
|
||||
let failure = await this.rollbackPublishedFailure(error)
|
||||
if (sandbox !== undefined && this.stateDirectoryCreated) {
|
||||
try {
|
||||
await this.removeFailedState(sandbox)
|
||||
@@ -413,7 +425,7 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
const files = [
|
||||
{ path: this.paths.pid, data: '' },
|
||||
{ path: this.paths.status, data: '' },
|
||||
{ path: this.paths.environment, data: remoteEnvironment(ambient.stdout, this.spec.env) },
|
||||
{ path: this.paths.environment, data: serializeRemoteEnvironment(ambient.stdout, this.spec.env) },
|
||||
...(hasSpill(this.spec.stdio.stdout) ? [{ path: this.paths.stdout, data: '' }] : []),
|
||||
...(hasSpill(this.spec.stdio.stderr) ? [{ path: this.paths.stderr, data: '' }] : []),
|
||||
]
|
||||
@@ -508,15 +520,16 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
if (!/^(?:0|[1-9][0-9]*)$/.test(rawStatus) || !Number.isSafeInteger(exitCode) || exitCode > 255) {
|
||||
throw new Error(`subprocess-e2b: remote wrapper published invalid exit code ${JSON.stringify(rawStatus)}`)
|
||||
}
|
||||
if (this.spec.stdio.stdout === 'pipe' || this.spec.stdio.stderr === 'pipe') {
|
||||
return this.commandOutcome(await settlement)
|
||||
}
|
||||
const completed = await withinMs(settlement, this.spec.graceMs)
|
||||
if (completed !== undefined) return this.commandOutcome(completed)
|
||||
this.outputDrainExpired = true
|
||||
this.stdoutReader?.invalidateSpill()
|
||||
this.stderrReader?.invalidateSpill()
|
||||
await handle.disconnect()
|
||||
return this.terminationSignal === null
|
||||
? { exitCode, signal: null }
|
||||
: { exitCode: null, signal: this.terminationSignal }
|
||||
return { exitCode, signal: null }
|
||||
}
|
||||
const completed = await Promise.race([settlement, waitTick().then(() => undefined)])
|
||||
if (completed !== undefined) return this.commandOutcome(completed)
|
||||
@@ -533,6 +546,20 @@ export class E2BSubprocessHandle implements SubprocessHandle {
|
||||
throw settlement.error
|
||||
}
|
||||
|
||||
private async rollbackPublishedFailure(error: unknown): Promise<unknown> {
|
||||
if (this.remotePid <= 0 || this.commandHandle === undefined || this.quiescenceProven) return error
|
||||
this.terminate()
|
||||
try {
|
||||
await this.waitForExit()
|
||||
return error
|
||||
} catch (cleanupError: unknown) {
|
||||
return new AggregateError(
|
||||
[asError(error), asError(cleanupError)],
|
||||
'subprocess-e2b: command monitoring failed and process-group rollback did not reach quiescence',
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private async rollbackUnpublishedGroup(sandbox: Sandbox, handle: CommandHandle): Promise<void> {
|
||||
// The bootstrap ends in an exec chain through the scrubbed environment and
|
||||
// `setsid`, so E2B's command PID is the provisional group id even before the
|
||||
|
||||
@@ -19,7 +19,7 @@ import type {
|
||||
SubprocessTerminalSpawnSpec,
|
||||
} from '@deepseek-ai/dsh-subprocess'
|
||||
import type E2BSandboxService from '@deepseek-ai/dsh-e2b'
|
||||
import { scrubRemoteEnvironment } from './environment.ts'
|
||||
import { serializeRemoteEnvironment } from './environment.ts'
|
||||
|
||||
const POLL_MS = 20
|
||||
|
||||
@@ -142,17 +142,6 @@ function serializeValues(values: readonly string[], kind: string): string {
|
||||
return values.map(value => `${value}\0`).join('')
|
||||
}
|
||||
|
||||
function remoteEnvironment(raw: string, explicit: Readonly<Record<string, string>> | undefined): string {
|
||||
const environment = scrubRemoteEnvironment(raw)
|
||||
for (const [name, value] of Object.entries(explicit ?? {})) {
|
||||
if (name.length === 0 || name.includes('=') || name.includes('\0') || value.includes('\0')) {
|
||||
throw new Error('subprocess-e2b: terminal environment entries require non-empty NUL-free names without = and NUL-free values')
|
||||
}
|
||||
environment.set(name, value)
|
||||
}
|
||||
return serializeValues([...environment].map(([name, value]) => `${name}=${value}`), 'environment')
|
||||
}
|
||||
|
||||
async function terminalSessionId(sandbox: Sandbox, pid: number, signal?: AbortSignal): Promise<number> {
|
||||
const result = await sandbox.commands.run(`ps -o sid= -p ${pid}`, signalOpts(signal))
|
||||
signal?.throwIfAborted()
|
||||
@@ -432,12 +421,14 @@ export class E2BTerminalHandle implements SubprocessTerminalHandle {
|
||||
* @param runtime - Shared E2B sandbox owner.
|
||||
* @param spec - Fully specified terminal-process request.
|
||||
* @param stateDir - Private remote directory for one startup transaction.
|
||||
* @param retainFailedCleanup - Optional owner for retrying a cleanup transaction that could not prove quiescence.
|
||||
* @returns The live subprocess terminal handle.
|
||||
*/
|
||||
export async function spawnE2BTerminal(
|
||||
runtime: E2BSandboxService,
|
||||
spec: SubprocessTerminalSpawnSpec,
|
||||
stateDir: string,
|
||||
retainFailedCleanup?: (cleanup: () => Promise<void>) => void,
|
||||
): Promise<E2BTerminalHandle> {
|
||||
const sandbox = await runtime.getSandbox()
|
||||
spec.signal?.throwIfAborted()
|
||||
@@ -456,7 +447,7 @@ export async function spawnE2BTerminal(
|
||||
let stateDirectoryCreated = false
|
||||
try {
|
||||
const ambient = await sandbox.commands.run('env -0', signalOpts(spec.signal))
|
||||
const environment = remoteEnvironment(ambient.stdout, spec.env)
|
||||
const environment = serializeRemoteEnvironment(ambient.stdout, spec.env)
|
||||
const argv = serializeValues(spec.argv, 'argv')
|
||||
await sandbox.files.makeDir(stateDir)
|
||||
stateDirectoryCreated = true
|
||||
@@ -502,25 +493,37 @@ export async function spawnE2BTerminal(
|
||||
)
|
||||
} catch (error: unknown) {
|
||||
output.destroy()
|
||||
const cleanupErrors: Error[] = []
|
||||
if (handle !== undefined && completion !== undefined) {
|
||||
try {
|
||||
await rollbackUnpublishedTerminal(sandbox, handle, completion, spec.graceMs)
|
||||
} catch (rollbackError: unknown) {
|
||||
cleanupErrors.push(asError(rollbackError))
|
||||
let terminalQuiescent = handle === undefined
|
||||
let stateRemoved = !stateDirectoryCreated
|
||||
const retryCleanup = async (): Promise<void> => {
|
||||
const failures: Error[] = []
|
||||
if (!terminalQuiescent && handle !== undefined) {
|
||||
try {
|
||||
if (completion === undefined) await handle.kill()
|
||||
else await rollbackUnpublishedTerminal(sandbox, handle, completion, spec.graceMs)
|
||||
terminalQuiescent = true
|
||||
} catch (cleanupError: unknown) {
|
||||
failures.push(asError(cleanupError))
|
||||
}
|
||||
}
|
||||
} else if (handle !== undefined) {
|
||||
await handle.kill().catch(() => false)
|
||||
}
|
||||
if (stateDirectoryCreated) {
|
||||
try {
|
||||
await sandbox.files.remove(stateDir)
|
||||
} catch (stateError: unknown) {
|
||||
if (!(stateError instanceof FileNotFoundError)) cleanupErrors.push(asError(stateError))
|
||||
if (!stateRemoved) {
|
||||
try {
|
||||
await sandbox.files.remove(stateDir)
|
||||
stateRemoved = true
|
||||
} catch (stateError: unknown) {
|
||||
if (stateError instanceof FileNotFoundError) stateRemoved = true
|
||||
else failures.push(asError(stateError))
|
||||
}
|
||||
}
|
||||
if (failures.length > 0) {
|
||||
throw new AggregateError(failures, 'subprocess-e2b: terminal setup cleanup did not complete')
|
||||
}
|
||||
}
|
||||
if (cleanupErrors.length > 0) {
|
||||
throw new AggregateError([asError(error), ...cleanupErrors], asError(error).message)
|
||||
try {
|
||||
await retryCleanup()
|
||||
} catch (cleanupError: unknown) {
|
||||
retainFailedCleanup?.(retryCleanup)
|
||||
throw new AggregateError([asError(error), asError(cleanupError)], asError(error).message)
|
||||
}
|
||||
throw error
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user