Skip to content

testing: Close POSIX result pipes on FIFO EOF - #26082

Draft
eleanorjboyd wants to merge 2 commits into
microsoft:mainfrom
eleanorjboyd:agents/vscode-python-issue-26071-feedback
Draft

testing: Close POSIX result pipes on FIFO EOF#26082
eleanorjboyd wants to merge 2 commits into
microsoft:mainfrom
eleanorjboyd:agents/vscode-python-issue-26071-feedback

Conversation

@eleanorjboyd

Copy link
Copy Markdown
Member

Summary

  • replace the POSIX FIFO net.Socket reader with a nonblocking reader that explicitly detects writer disconnect and emits reader completion
  • preserve buffered result delivery during cancellation while cleaning up FIFO descriptors and files exactly once
  • return an owned result-pipe handle so pytest and unittest adapters dispose resources after the drain timeout fallback
  • add real POSIX FIFO coverage for buffered payloads, empty writers, cancellation, and adapter cleanup

Why

A net.Socket created from a nonblocking FIFO descriptor receives data but does not emit end or close when the FIFO writer exits. After the cancellation drainage change, test execution therefore waited for the five-second fallback on every affected run.

The new reader drains the descriptor directly and settles onClose after EOF, avoiding the delay without reintroducing dropped buffered results.

Fixes #26071

eleanorjboyd and others added 2 commits July 31, 2026 10:42
Drain nonblocking FIFO readers to EOF and explicitly dispose result pipes after the fallback timeout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the disposable result pipe handle returned by the updated test utility.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a POSIX FIFO EOF detection gap in the test result named-pipe reader that caused test runs to regularly wait for the 5s RESULT_PIPE_DRAIN_TIMEOUT_MS fallback after the Python subprocess exited. It replaces the FIFO net.Socket reader with a direct FD reader that can reliably settle onClose on writer disconnect, and updates adapters to explicitly dispose result-pipe resources after the drain/timeout window.

Changes:

  • Replace the POSIX FIFO reader implementation to detect writer EOF and fire close reliably (FifoMessageReader).
  • Change startRunResultNamedPipe() to return an owned { name, dispose } handle and have pytest/unittest execution adapters dispose it after drain/timeout.
  • Update/extend unit tests to cover POSIX FIFO reader behavior and adapter disposal during cancellation.
Show a summary per file
File Description
src/client/common/pipes/namedPipes.ts Adds a POSIX FIFO FD-based message reader and wires it into createReaderPipe().
src/client/testing/testController/common/utils.ts Changes startRunResultNamedPipe() to return an owned handle with a dispose() method.
src/client/testing/testController/pytest/pytestExecutionAdapter.ts Uses resultPipe.name and disposes the result pipe after drain/timeout.
src/client/testing/testController/unittest/testExecutionAdapter.ts Uses resultPipe.name and disposes the result pipe after drain/timeout.
src/test/common/pipes/namedPipes.unit.test.ts Adds POSIX FIFO tests for buffered payloads, empty writer, and cancellation close.
src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts Updates stubs to match the new { name, dispose } result-pipe return type.
src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts Updates stubs to match the new { name, dispose } result-pipe return type.
src/test/testing/testController/testCancellationRunAdapters.unit.test.ts Asserts the adapter disposes the result-pipe handle during cancellation flows.

Review details

  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +197 to +201
if (bytesRead > 0) {
this.hasReadData = true;
this.stream.write(Buffer.from(this.buffer.subarray(0, bytesRead)));
continue;
}
@eleanorjboyd eleanorjboyd added the bug Issue identified by VS Code Team member as probable bug label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test Explorer always waits an extra ~5s per run: named-pipe "drain" deferred never resolves, falls back to RESULT_PIPE_DRAIN_TIMEOUT_MS

2 participants