From 995f449703a4977dc6bb4706736f2fc3114f9f17 Mon Sep 17 00:00:00 2001 From: Wasim Date: Sat, 17 Oct 2026 16:11:08 +0530 Subject: [PATCH] fix(testing): resolve result pipe drain promptly on cancellation Fixes #26217 On non-Windows platforms (especially macOS), non-blocking FIFO sockets do not emit an end or close event when the python subprocess writer exits. Because the server close deferred previously relied exclusively on reader.onClose after PR #25981, test runs always waited for the full RESULT_PIPE_DRAIN_TIMEOUT_MS (5 seconds) to expire. This change starts a short drain grace timer (100ms) upon cancellation or subprocess completion (via cancellationToken.onCancellationRequested) so buffered messages can finish draining while ensuring the result pipe resolves promptly without hanging on systems where reader.onClose does not fire. --- .../testing/testController/common/utils.ts | 30 +++++++++++++++++-- .../testing/testController/utils.unit.test.ts | 22 ++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 583d57648548..7d714253a182 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -33,6 +33,11 @@ export function createTestingDeferred(): Deferred { // platform-specific named-pipe quirks) so the adapter's `finally` block can never hang. export const RESULT_PIPE_DRAIN_TIMEOUT_MS = 5_000; +// Grace period (ms) to allow buffered results to drain after cancellation +// or subprocess termination before closing the result pipe when `reader.onClose` +// does not fire (such as non-blocking FIFO sockets on macOS). +export const RESULT_PIPE_DRAIN_GRACE_PERIOD_MS = 100; + /** * Awaits `deferred.promise` but resolves after at most `timeoutMs` so callers * cannot hang indefinitely if the underlying event source never fires. @@ -119,18 +124,39 @@ export async function startRunResultNamedPipe( traceVerbose('Starting Test Result named pipe'); const pipeName: string = generateRandomPipeName('python-test-results'); - // `cancellationToken` only cancels pipe creation; disposal is driven by - // `reader.onClose` so buffered results are not dropped on cancel. + // `cancellationToken` cancels pipe creation immediately. On cancellation or subprocess + // completion, a brief drain window is started so buffered messages are delivered before + // disposal when `reader.onClose` never fires (e.g. non-blocking FIFOs on macOS). const reader = await createReaderPipe(pipeName, cancellationToken); traceVerbose(`Test Results named pipe ${pipeName} connected`); + let drainTimer: NodeJS.Timeout | undefined; let disposables: Disposable[] = []; const disposable = new Disposable(() => { + if (drainTimer) { + clearTimeout(drainTimer); + drainTimer = undefined; + } traceVerbose(`Test Results named pipe ${pipeName} disposed`); disposables.forEach((d) => d.dispose()); disposables = []; deferredTillServerClose.resolve(); }); + if (cancellationToken) { + const scheduleDrain = () => { + if (!drainTimer) { + drainTimer = setTimeout(() => { + disposable.dispose(); + }, RESULT_PIPE_DRAIN_GRACE_PERIOD_MS); + } + }; + if (cancellationToken.isCancellationRequested) { + scheduleDrain(); + } else { + disposables.push(cancellationToken.onCancellationRequested(scheduleDrain)); + } + } + disposables.push( reader, reader.listen((data: Message) => { diff --git a/src/test/testing/testController/utils.unit.test.ts b/src/test/testing/testController/utils.unit.test.ts index 0fae5ffbcee2..fe177c14bf8d 100644 --- a/src/test/testing/testController/utils.unit.test.ts +++ b/src/test/testing/testController/utils.unit.test.ts @@ -935,6 +935,28 @@ suite('startRunResultNamedPipe drain-on-cancel tests', () => { assert.strictEqual(reader.disposed, true, 'reader disposed when onClose fires'); }); + + test('cancellation with reader that never closes resolves deferredTillServerClose after drain grace period', async () => { + const reader = new FakeMessageReader(); + sandbox.stub(namedPipes, 'createReaderPipe').resolves(reader); + + const received: ExecutionTestPayload[] = []; + const deferredTillServerClose: Deferred = createDeferred(); + const cancelSource = new CancellationTokenSource(); + + await startRunResultNamedPipe((payload) => received.push(payload), deferredTillServerClose, cancelSource.token); + + cancelSource.cancel(); + // Before grace period expires, not yet resolved + await new Promise((r) => setImmediate(r)); + assert.strictEqual((deferredTillServerClose as Deferred).completed, false); + + // After grace period expires, resolved and reader disposed + await deferredTillServerClose.promise; + assert.strictEqual(reader.disposed, true); + + cancelSource.dispose(); + }); }); suite('awaitDeferredWithTimeout', () => {