diff --git a/src/main/providers/ssh-pty-spawn-exit-race.test.ts b/src/main/providers/ssh-pty-spawn-exit-race.test.ts index 4a911e4b8fb..19624dd55df 100644 --- a/src/main/providers/ssh-pty-spawn-exit-race.test.ts +++ b/src/main/providers/ssh-pty-spawn-exit-race.test.ts @@ -30,3 +30,72 @@ it('does not release a quarantined exit when one operation finishes twice', () = expect(publish).not.toHaveBeenCalled() }) + +it('releases a held exit when a late binder without an incarnation cannot attribute it', () => { + const tracker = new SshPtySpawnExitRaceTracker() + const publish = vi.fn() + const holder = tracker.begin('pty-1') + const late = tracker.begin() + + // Relays that send no incarnation id leave both sides unidentified, so nothing is attributable. + tracker.recordExit('pty-1', undefined, publish) + tracker.bind(late, 'pty-1') + expect(tracker.classifyPendingExit(late, { id: 'pty-1' })).toBe('unverifiable') + tracker.finish(late) + tracker.finish(holder) + + expect(publish).toHaveBeenCalledOnce() +}) + +it('releases a held exit when a late binder attaches without an incarnation', () => { + const tracker = new SshPtySpawnExitRaceTracker() + const publish = vi.fn() + const holder = tracker.begin('pty-1') + const late = tracker.begin() + + tracker.recordExit('pty-1', 'incarnation-old', publish) + tracker.bind(late, 'pty-1') + expect(tracker.classifyPendingExit(late, { id: 'pty-1' })).toBe('unverifiable') + tracker.finish(late) + tracker.finish(holder) + + expect(publish).toHaveBeenCalledOnce() +}) + +it('publishes an unheld exit once even after a late binder attributes it', () => { + const tracker = new SshPtySpawnExitRaceTracker() + const publish = vi.fn() + const late = tracker.begin() + + // The notification router publishes whatever the tracker does not take ownership of. + if (!tracker.recordExit('pty-1', 'incarnation-1', publish)) { + publish() + } + tracker.bind(late, 'pty-1') + expect(tracker.classifyPendingExit(late, { id: 'pty-1', incarnationId: 'incarnation-1' })).toBe( + 'exited' + ) + tracker.finish(late) + + expect(publish).toHaveBeenCalledOnce() +}) + +it('cannot recall an exit released before a late binder attributes it', () => { + const tracker = new SshPtySpawnExitRaceTracker() + const publish = vi.fn() + const holder = tracker.begin('pty-1') + const late = tracker.begin() + + tracker.recordExit('pty-1', 'incarnation-1', publish) + // holders reaches zero before the late binder reaches its verdict, so the exit escapes. + tracker.finish(holder) + expect(publish).toHaveBeenCalledOnce() + + tracker.bind(late, 'pty-1') + expect(tracker.classifyPendingExit(late, { id: 'pty-1', incarnationId: 'incarnation-1' })).toBe( + 'exited' + ) + tracker.finish(late) + + expect(publish).toHaveBeenCalledOnce() +}) diff --git a/src/main/providers/ssh-pty-spawn-exit-race.ts b/src/main/providers/ssh-pty-spawn-exit-race.ts index 9ac2fbceebd..859868ea1bb 100644 --- a/src/main/providers/ssh-pty-spawn-exit-race.ts +++ b/src/main/providers/ssh-pty-spawn-exit-race.ts @@ -32,6 +32,7 @@ export class SshPtySpawnExitRaceTracker { operation.relayPtyId = relayPtyId } + /** True when the tracker owns publication of this exit: deferred to a holder, or already sent. */ recordExit(relayPtyId: string, incarnationId: unknown, publish?: () => void): boolean { let published = false const quarantine: QuarantinedSshPtyExit = { @@ -60,7 +61,15 @@ export class SshPtySpawnExitRaceTracker { ...(tracked ? { held, quarantine } : {}) }) } - return quarantine.holders > 0 + if (publish === undefined) { + return false + } + if (quarantine.holders === 0) { + // Why: no operation can fence this exit, so send it now — through the quarantine, so a + // later binder that attributes it cannot deliver the same death a second time. + quarantine.publish() + } + return true } classifyPendingExit( @@ -69,8 +78,14 @@ export class SshPtySpawnExitRaceTracker { ): SshPtyPendingExitOutcome { const sameIdExits = operation.exits.filter((exit) => exit.relayPtyId === result.id) for (const exit of sameIdExits) { - // Why: a reached verdict owns the exit, so finish() must not second-guess it and re-publish. - if (exit.quarantine) { + // Why: classifying claims the exit and silences its holder's release, so only claim what this + // operation actually attributed — it owned the PTY when the exit arrived, or both incarnations + // are known and comparable. A late binder that cannot compare identities has proven nothing, + // and stamping its "I can't tell" would strand a death the host reported. + const attributed = + exit.held === true || + (result.incarnationId !== undefined && exit.incarnationId !== undefined) + if (exit.quarantine && attributed) { exit.quarantine.classified = true } }