2031881 - Consider adding nsIAsync{Input,Output}Stream implementations for OS pipes

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

In our IPC code, we have an event loop for interacting with system level pipes/sockets in an async manner, but this is not really exposed to other consumers. This bug is to consider exposing this through XPCOM as a series of nsIAsync{Input,Output}Stream types.

The aim here is to make it easier for other components to take advantage of the async runtime we already use for our IPC backend.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 2031881 - Add new PlatformPipeReader type, r=jld!,#xpcom-reviewers!

Comment 2

[Greg Stoll :gstoll]

Attachment: WIP: TEMP Bug 2031881 - Add new AsyncPlatformPipes types r=nika!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/68c7dd23b59b
https://hg.mozilla.org/integration/autoland/rev/9bd9a2622456
Add new PlatformPipeReader type, r=jld

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a1d80969aac1
https://hg.mozilla.org/integration/autoland/rev/59dc69c853ad
Revert "Bug 2031881 - Add new PlatformPipeReader type, r=jld" for causing build bustages at /AsyncPlatformPipes

Comment 5

[Silaghi Andreea]

Backed out for causing build bustages at /AsyncPlatformPipes
Backout Link
Push with failures
Failure Log
Failure line ../../../../checkouts/gecko/xpcom/io/AsyncPlatformPipes.cpp:X:23: error: logical 'or' of equal expressions [-Werror=logical-op]

Comment 6

[Nika Layzell [:nika] (ni? for response)]

This is a simple fix - I was missing an #if EWOULDBLOCK != EAGAIN check which is only necessary on old C++ compilers.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b9ca38bc933e
https://hg.mozilla.org/integration/autoland/rev/9c88e1faac2f
Add new PlatformPipeReader type, r=jld

Comment 8

[amarc]

https://hg.mozilla.org/mozilla-central/rev/9c88e1faac2f

Comment 9

[Greg Stoll :gstoll]

Attachment: Bug 2031881 - Add new PlatformPipeReader type,

This is a new API which is intended to be used by code which wants to
perform async reads from native filesystem pipes. This does not support
async reads from "regular" files, as those are not compatible with
epoll.

Currently the API uses an owned buffer which is filled on the IPC I/O
thread, and read from on the user's thread. This is technically only
necessary for Windows (as non-Windows OSes could directly invoke read
during nsIInputStream::read method), This is because overlapped I/O on
Windows requires that the buffer be kept alive throughout the async
operation. So far I have made the call to always use the buffer for API
support consistency, but we could potentially change that behaviour.

The actual system file handle management is largely isolated to the
PlatformPipeLink type within the .cpp file. This is done to allow the
lifecycle of the nsIInputStream instance to be disconnected from the
lifecycle of the underlying pipe, as things like the buffer need to be
kept alive potentially longer than the stream to avoid issues on Windows
& handle cancelling I/O on destruction.

Currently there is no way to use this type with a shared file handle,
the file handle must be owned by the PlatformPipeLink.

Previous versions of this patch contained a PlatformPipeWriter type as
well, but that had some issues with lifecycle which I haven't yet
figured out yet, and turns out to not be necessary for the WebSerial
work right now, so are being left to a follow-up.

Original Revision: https://phabricator.services.mozilla.com/D294317

Comment 10

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/e7bbe68b642d
https://hg.mozilla.org/releases/mozilla-beta/rev/31f68296aa55