Skip to content

Use overlapped pipes in WSLC session terminate tests#40441

Open
benhillis wants to merge 1 commit intomasterfrom
fix/wslc-overlapped-pipe-resubmit
Open

Use overlapped pipes in WSLC session terminate tests#40441
benhillis wants to merge 1 commit intomasterfrom
fix/wslc-overlapped-pipe-resubmit

Conversation

@benhillis
Copy link
Copy Markdown
Member

The LoadImage and ImportImage 'session terminate' sub-tests used CreatePipe which creates synchronous pipe handles. When the relay calls ReadFile with an OVERLAPPED structure on a synchronous handle, the call blocks the thread instead of returning ERROR_IO_PENDING. This prevents WaitForMultipleObjects from ever checking the session terminating event, causing a deadlock when Terminate() is called.

Replace CreatePipe with OpenAnonymousPipe (FILE_FLAG_OVERLAPPED) so ReadFile returns ERROR_IO_PENDING and the relay's event loop can detect the termination signal.

The LoadImage and ImportImage 'session terminate' sub-tests used CreatePipe
which creates synchronous pipe handles. When the relay calls ReadFile with an
OVERLAPPED structure on a synchronous handle, the call blocks the thread
instead of returning ERROR_IO_PENDING. This prevents WaitForMultipleObjects
from ever checking the session terminating event, causing a deadlock when
Terminate() is called.

Replace CreatePipe with OpenAnonymousPipe (FILE_FLAG_OVERLAPPED) so ReadFile
returns ERROR_IO_PENDING and the relay's event loop can detect the
termination signal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 6, 2026 16:34
@benhillis benhillis requested a review from a team as a code owner May 6, 2026 16:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 deadlock in the WSLC “session terminate” sub-tests for LoadImage and ImportImage by ensuring the test input pipes support overlapped I/O, allowing the relay loop’s WaitForMultipleObjects-based cancellation to observe the session termination signal.

Changes:

  • Replace CreatePipe with wsl::windows::common::wslutil::OpenAnonymousPipe(..., ReadPipeOverlapped=true, ...) in the LoadImage session-terminate test.
  • Apply the same overlapped-pipe change to the ImportImage session-terminate test.
  • Add clarifying comments explaining why overlapped pipes are required for these specific sub-tests.

Copy link
Copy Markdown
Collaborator

@OneBlue OneBlue left a comment

Choose a reason for hiding this comment

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

This prevents WaitForMultipleObjects from ever checking the session terminating event, causing a deadlock when Terminate() is called

I don't think this is true. We specifically have code to cancel synchronous IO when the session terminates. Did we see a deadlock caused by this test ?

@benhillis
Copy link
Copy Markdown
Member Author

This prevents WaitForMultipleObjects from ever checking the session terminating event, causing a deadlock when Terminate() is called

I don't think this is true. We specifically have code to cancel synchronous IO when the session terminates. Did we see a deadlock caused by this test ?

This assertion is from copilot, I don't buy it either. However, I think switching away from the CreatePipe API is probably a good call.

@OneBlue
Copy link
Copy Markdown
Collaborator

OneBlue commented May 6, 2026

This assertion is from copilot, I don't buy it either. However, I think switching away from the CreatePipe API is probably a good call.

Ideally I think we should test with both (the BlockingOperations tests use overlapped pipes), so we can validate that we behave correctly in both cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants