Add CommunicationHandle, an API for inter-process communication via Handles

[sheaf]

NB This PR depends on #310. For review, please only look at the last commit (at the time of writing, this was a6af24e).

This PR adds the System.Process.CommunicationHandle module, which provides the cross-platform CommunicationHandle abstraction which allows Handles to be passed to child processes for inter-process communication.

A high-level API is provided by the function readCreateProcessWithExitCodeCommunicationHandle, which can be consulted for further details about how the functionality is meant to be used.

To test this functionality, I added the new cli-child executable component to the library. To work around Cabal bug #9854, it was necessary to change the build-type of the library to Custom, in order to make the cli-child executable visible when running the test-suite. The custom Setup.hs script contains more details about the problem.

TODO:

• On Windows using WinIO, associating the parent handles to the parent process triggers associateHandleWithIOCP: invalid argument (The parameter is incorrect.). (To reproduce, run cabal run test -- +RTS -io-manager=native; comment out all tests except testCommunicationHandle in test/main.hs for convenience.)
  • Fixed by properly creating a non-inheritable handle for the parent.
• On Windows, when NOT using WinIO, if we change the child from let !output = force $ (reverse $ take 5 input) ++ "123" to let !output = force $ (reverse input) ++ "123", i.e. waiting for the end of the stream, then the test loops indefinitely.
  • Fixed by using named pipe always, instead of using anonymous pipes when not using WinIO.
• Get CI working: done, see Move tests to separate process-tests package & update CI to use GitHub actions #310.

[sheaf]

@Mistuke I have mostly taken the implementation from the following two modules in the GHC source code:

• GHC.Runtime.Utils.runWithPipes for the parent process,
• GHCi.Utils for the child process.

This PR is intended to replace all that code, providing an API for other consumers than just GHC.

I have attempted to debug the two TODOs described in the OP with @bgamari. In particular, we hope you'd be able to comment about the WinIO-specific aspects; in particular, it seems that creating a pipe (using the process createPipe function) and then immediately associating one end with the parent using associateHandle' seems to trigger associateHandleWithIOCP: invalid argument (The parameter is incorrect.), whereas associating the other end after it has been inherited by the child seems to work OK. I took that code from here... is that code path currently tested?

[sheaf]

I fixed the first bug, which is the same bug as GHC #24618 (as I got it from copying the code from GHC.Runtime.Utils.runWithPipes). Now we correctly only set as inheritable the side of the pipe we pass to the child, and we don't call associateHandle' in the child on the inherited handle.

The second bug is likely a bug in the compatibility unix code for Windows, but I haven't investigated.

[bgamari]

@sheaf the Windows issues when using the old IO manager may indeed be attributable due to an infelicity in msvcrt's POSIX IO implementation, which is known to be suspect, as the logic in this MR works on both POSIX platforms and under WinIO.

In my own local testing I have confirmed that close is called on parent's write handle yet the child remains blocked on its read handle.

[sheaf]

@Mistuke Would it be possible for you to review the new System.Process.CommunicationHandle module? As mentioned above, it's based on the code in GHC.Runtime.Utils.runWithPipes (for the parent process) and GHCi.Utils (for the child process). It's intended to replace that code, fixing GHC bug #24618 in the process.

[Mistuke]

@Mistuke Would it be possible for you to review the new System.Process.CommunicationHandle module? As mentioned above, it's based on the code in GHC.Runtime.Utils.runWithPipes (for the parent process) and GHCi.Utils (for the child process). It's intended to replace that code, fixing GHC bug #24618 in the process.

So first thing to note, is that you cannot use async APIs (even in synchronous mode) with anonymous pipes.
So if the handle is created with _pipe there's no way for WinIO to handle it.

In general a consumer cannot assume anything about the handle it received. If the receiver wants to make it an async handle it has to do so. i.e. it has to call ReOpenFile https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reopenfile?redirectedfrom=MSDN with the flag it requires.

i.e.

HANDLE hwout = ReOpenFile(hwin, GENERIC_READ | GENERIC_WRITE,
                                                 FILE_SHARE_READ, FILE_FLAG_OVERLAPPED);

But again, this does not work at all for anonymous pipes. This is why for WinIO process creates Named Pipes through mkNamedPipe. So the code is indeed intended to work for WINIO -> WINIO.

note that WinIO can handle synchronous handles without them being associated with it. the ReadFile call just becomes blocking

If hFile is not opened with FILE_FLAG_OVERLAPPED and lpOverlapped is not NULL, the read operation starts
at the offset specified in the OVERLAPPED structure. ReadFile does not return until the read operation has
been completed.

It means your I/O throughput will drop but it should still work.

[Mistuke]

This looks on the whole good to me, note that this never made it into the docs but explains some of the behavioral difference #235 (comment)

I do have one question, since you're making a new API, I wonder, should make the distinction between HANDLE and FDs? I think it would be better to only use HANDLE. i.e. only have usingPipes. Named pipes should be readable through the same interfaces as anonymous ones. It's the inverse that's not true.

[sheaf]

So first thing to note, is that you cannot use async APIs (even in synchronous mode) with anonymous pipes.
So if the handle is created with _pipe there's no way for WinIO to handle it.

What I don't understand is that, with the first version of this PR, it worked fine for the parent to not use WinIO yet for the child to use WinIO. The parent will use createPipeFd (i.e. _pipe and not mkNamedPipe), call _get_osfhandle on that to get a HANDLE, pass that to the child, and on the other end (now using WinIO) we call getGhcHandleNative :: HANDLE -> IO Handle and we are able to read/write from the handle.

I do have one question, since you're making a new API, I wonder, should make the distinction between HANDLE and FDs? I think it would be better to only use HANDLE. i.e. only have usingPipes. Named pipes should be readable through the same interfaces as anonymous ones. It's the inverse that's not true.

I have pushed a commit that uses only mkNamedPipe and not _pipe. See createCommunicationPipe. However I'm not sure how to use the HANDLE that mkNamedPipe creates to create a Handle on a GHC that doesn't have WinIO support (i.e. I need the System.Process.Common.mbPipeHANDLE function even on a GHC that doesn't support WinIO). Currently I am doing open_osfhandle :: HANDLE -> IO Fd, mkFd :: Fd -> ... -> IO FD, mkFileHandle :: FD -> ... -> IO Handle, but I don't know if that is throwing the baby out with the bathwater.

[sheaf]

Using mkNamedPipe has fixed the second bug reported in the OP, wherein the child process would hang waiting for the end of the stream when the parent was not using WinIO.

[sheaf]

I think this is good to go now. I was just waiting for the lower bounds to the time package to be updated, as incorrect bounds were causing CI to fail. CI should be reliably green now.