Skip to content

event: support pipe sources on Windows #501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2019
Merged

event: support pipe sources on Windows #501

merged 1 commit into from
Jun 27, 2019

Conversation

adierking
Copy link
Contributor

This implements full support for using pipe sources on Windows to
trigger an event when a pipe becomes readable or writable.

Due to differences between the Windows asynchronous I/O model and the
Unix model, this is rather complex. On Windows, the standard mechanism
to achieve asynchronous pipe I/O is to just read or write from the pipe
and then get notified once the operation completes. Unlike the Unix
select()/poll() model, there is no way to simply know when a pipe
becomes readable or writable without actually running an operation. So
we need to resort to several tricks in order to achieve the semantics
that Dispatch wants here.

To monitor a pipe for readability, we take advantage of the fact that a
zero-byte ReadFile() on a pipe will block until data becomes available
in the pipe. A muxnote which monitors a pipe for reading will spin up a
lightweight thread which repeatedly calls ReadFile() (blocking) on the
pipe and posts back to the I/O completion queue when it returns.

To monitor pipes for writability, we use the NtQueryInformationFile()
kernel API to get the amount of available space in the pipe's write
buffer. There is no way to block here, so we have no choice but to
continually disarm and rearm unotes until space becomes available. This
is inefficient, but it generally seems to work OK.

In order to test this, I implemented a new dispatch_io_pipe test which
performs various read and write operations on pipes. On Windows, this
will run the tests on most of the different pipe kinds (anonymous,
named, inbound, outbound, overlapped). This caught a lot of issues in
the Windows _dispatch_operation_perform() which I fixed along the way.
The dispatch_io and dispatch_io_pipe_close tests pass as well with my
other pull request applied.

@adierking
Copy link
Contributor Author

cc @compnerd @ktopley-apple

Copy link
Contributor

@ktopley-apple ktopley-apple left a comment

Choose a reason for hiding this comment

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

Most of this code is Windows-specific and I do not have the expertise required to review it in detail. I have confined my comments to the parts that are not Windows-specific.

@ktopley-apple
Copy link
Contributor

@swift-ci please test

This implements full support for using pipe sources on Windows to
trigger an event when a pipe becomes readable or writable.

Due to differences between the Windows asynchronous I/O model and the
Unix model, this is rather complex. On Windows, the standard mechanism
to achieve asynchronous pipe I/O is to just read or write from the pipe
and then get notified once the operation completes. Unlike the Unix
`select()`/`poll()` model, there is no way to simply know when a pipe
becomes readable or writable without actually running an operation. So
we need to resort to several tricks in order to achieve the semantics
that Dispatch wants here.

To monitor a pipe for readability, we take advantage of the fact that a
zero-byte `ReadFile()` on a pipe will block until data becomes available
in the pipe. A muxnote which monitors a pipe for reading will spin up a
lightweight thread which repeatedly calls `ReadFile()` (blocking) on the
pipe and posts back to the I/O completion queue when it returns.

To monitor pipes for writability, we use the `NtQueryInformationFile()`
kernel API to get the amount of available space in the pipe's write
buffer. There is no way to block here, so we have no choice but to
continually disarm and rearm unotes until space becomes available. This
is inefficient, but it generally seems to work OK.

In order to test this, I implemented a new dispatch_io_pipe test which
performs various read and write operations on pipes. On Windows, this
will run the tests on most of the different pipe kinds (anonymous,
named, inbound, outbound, overlapped). This caught a lot of issues in
the Windows `_dispatch_operation_perform()` which I fixed along the way.
The dispatch_io and dispatch_io_pipe_close tests pass as well with my
other pull request applied.
@adierking
Copy link
Contributor Author

Fixed the build failure in Release mode and a merge conflict that arose. cc @ktopley-apple

@ktopley-apple
Copy link
Contributor

@swift-ci please test

1 similar comment
@compnerd
Copy link
Member

@swift-ci please test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me. I think that we should merge this to get at least a basis for Windows. Some local preliminary testing shows that this significantly improves the state of Foundation tests.

@@ -103,17 +136,28 @@ _dispatch_muxnote_create(dispatch_unote_t du)
// The specified file is a character file, typically a
// LPT device or a console.
WIN_PORT_ERROR();
free(dmn);
return NULL;
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this fix.

// We'll get ERROR_ACCESS_DENIED for outbound pipes.
if (GetLastError() != ERROR_ACCESS_DENIED) {
// The file is probably a socket.
WIN_PORT_ERROR();
Copy link
Member

Choose a reason for hiding this comment

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

hopefully SOCKETs end up being easier than pipes.

@ktopley-apple ktopley-apple merged commit d11d565 into swiftlang:master Jun 27, 2019
rokhinip pushed a commit that referenced this pull request Nov 5, 2021
event: support pipe sources on Windows
Signed-off-by: Kim Topley <[email protected]>
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.

3 participants