Support async NamedPipeServerStream.WaitForConnection for synchronous pipes

[GSPP]

Motivating use case: My codebase is using synchronous pipe IO for IPC. The application spends a lot of CPU in the IPC mechanism, and synchronous IO uses less CPU than async IO. Thread count is not an issue, and synchronous code is simpler than async code. Sync IO is an appropriate solution.

I only need async for the WaitForConnection call because the child process might die and never connect. A timeout is not possible. Instead, I'm listening for process exit to terminate the pipe.

A synchronous WaitForConnection call is really hard to break. For example, closing the pipe handle (Dispose) does not unblock that call... Probably, CancelSynchronousIo could do it, but I read in another issue that this is hard to implement.

WaitForConnectionAsync works on a synchronous pipe, but it simply delegates the sync call to the thread-pool with no ability to cancel. That call will hang forever. I wonder if this implicit async-over-sync design might be a foot gun for people.

Currently, the only workaround that I know of is to create a dummy client pipe and connect to the server to break the WaitForConnection call. This scheme is not the nicest solution in the world, and I wonder if there are circumstances where it would break (security?).

What would it take to support this? Does the Win32 API not support overlapped IO on pipes if the pipe mode is synchronous? I could not find such a statement in the documentation. But if that is the case, SetNamedPipeHandleMode can temporarily switch the mode to async. Is this feasible to support? This behavior would then be transparent to callers of WaitForConnectionAsync.

An alternative solution is to make IsAsync settable. ReadMode already is settable, so the infrastructure and precedence for calling SetNamedPipeHandleMode are there.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Motivating use case: My codebase is using synchronous pipe IO for IPC. The application spends a lot of CPU in the IPC mechanism, and synchronous IO uses less CPU than async IO. Thread count is not an issue, and synchronous code is simpler than async code. Sync IO is an appropriate solution.

I only need async for the WaitForConnection call because the child process might die and never connect. A timeout is not possible. Instead, I'm listening for process exit to terminate the pipe.

A synchronous WaitForConnection call is really hard to break. For example, closing the pipe handle (Dispose) does not unblock that call... Probably, CancelSynchronousIo could do it, but I read in another issue that this is hard to implement.

WaitForConnectionAsync works on a synchronous pipe, but it simply delegates the sync call to the thread-pool with no ability to cancel. That call will hang forever. I wonder if this implicit async-over-sync design might be a foot gun for people.

Currently, the only workaround that I know of is to create a dummy client pipe and connect to the server to break the WaitForConnection call. This scheme is not the nicest solution in the world, and I wonder if there are circumstances where it would break (security?).

What would it take to support this? Does the Win32 API not support overlapped IO on pipes if the pipe mode is synchronous? I could not find such a statement in the documentation. But if that is the case, SetNamedPipeHandleMode can temporarily switch the mode to async. Is this feasible to support? This behavior would then be transparent to callers of WaitForConnectionAsync.

An alternative solution is to make IsAsync settable. ReadMode already is settable, so the infrastructure and precedence for calling SetNamedPipeHandleMode are there.

Author:	GSPP
Assignees:	-
Labels:	area-System.IO, untriaged
Milestone:	-

[stephentoub]

Does the Win32 API not support overlapped IO on pipes if the pipe mode is synchronous?

Right:
https://docs.microsoft.com/en-us/windows/win32/api/namedpipeapi/nf-namedpipeapi-connectnamedpipe

If hNamedPipe was not opened with FILE_FLAG_OVERLAPPED, the function does not return until a client is connected or an error occurs. Successful synchronous operations result in the function returning a nonzero value if a client connects after the function is called.

SetNamedPipeHandleMode can temporarily switch the mode to async

Do you mean SetNamedPipeHandleState? That doesn't let you change whether the pipe was opened for overlapped I/O.

[fbrosseau]

Overlapped mode is forever because it is fundamental to a given file object.

Further, the non-blocking mode (specific to named pipes) you are referring to is obsolete: (quote from Win32 ConnectNamedPipe):

Note Nonblocking mode is supported for compatibility with Microsoft LAN Manager version 2.0, and it should not be used to achieve asynchronous input and output (I/O) with named pipes.

Then,

The application spends a lot of CPU in the IPC mechanism, and synchronous IO uses less CPU than async IO

While technically slightly true, I find it surprising that you would see noticeable overhead with async versus sync here? You are certainly right that sync is simpler code than async, but unless your application consists of two processes each having a single thread sending as many bytes as possible without actually processing them, I would assume the performance of that part to be negligible in the big picture outside of micro-benchmarks.

Also, if it is not just 1:1 threads spamming each other, but instead, multiple such connections concurrently, I would expect the async model to behave better than the sync model at extreme scale, since the async model can result in less context switching. But even having said this, I would be surprised it would actually make any noticeable difference in a real-world application that actually does anything with the data being exchanged.

All this to say, I think it would be worth biting the bullet and having it async : ).

[GSPP]

Thanks for the clarifications that you made.

The two processes are communicating through a BinaryReader/Writer based protocol. Performance is based not on the number of bytes but on the number of fields that are being exchanged. I'm using BufferedStream to call into the OS rarely, but at the reader/writer level the calls would need to be async still. The whole codebase is synchronous and making it async would be purely detrimental with zero value added, besides the WaitForConnect problem being solved.

It seems that my suggestion of using SetNamedPipeHandleState would not work. That's unfortunate.

That leaves only CancelIo* as a strategy to support canceling WaitForConnection. CancelIoEx can be used to cancel synchronous IO associated with a given handle. Unfortunately, I believe there is an unfixable race condition in that cancellation might be requested before the IO is started.

I was able to make cancellation work with CancelSynchronousIo by first closing the pipe. This will deny any future WaitForConnection calls. After the handle is closed, pending IOs can be canceled by specifying the thread. Without closing the pipe, I'm not sure how to fix the race.

It's unfortunate that Windows requires the IO mode to be set for the handle, and it's permanent. It would be convenient if each operation could choose what mode it wants to use. Is there a reason this design was chosen by Windows (decades ago)?

[adamsitnik]

@GSPP one more thing that you could try would be the following:

1. Open NamedPipeServerStream for async file IO
2. Use WaitForConnectionAsync with cancellation token
3. Once the client connects, use ReOpenFile sys-call and try to re-open the file for sync IO.
4. Create new NamedPipeServerStream out of the sync handle.

Something like this:

async Task<NamedPipeServerStream> Test(CancellationToken token)
{
    const uint FILE_GENERIC_READ = 0x80000000;
    const uint FILE_GENERIC_WRITE = 0x40000000;

    NamedPipeServerStream asyncStream = new ("test", PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous);
    await asyncStream.WaitForConnectionAsync(token);
    SafePipeHandle syncHandle = ReOpenFile(asyncStream.SafePipeHandle, FILE_GENERIC_READ | FILE_GENERIC_WRITE, (uint)FileShare.Read, (uint)FileOptions.None);
    // when should the handle be disposed?!
    if (syncHandle.IsInvalid)
    {
        int error = Marshal.GetLastPInvokeError();
        throw new Exception($"ReOpenFile failed with {error}");
    }
    return new NamedPipeServerStream(PipeDirection.InOut, isAsync: false, isConnected: true, syncHandle);
}

[DllImport("kernel32.dll", SetLastError = true)]
public static extern SafePipeHandle ReOpenFile(SafePipeHandle hOriginalFile, uint dwAccess, uint dwShareMode, uint dwFlags);

Usually mixing sync and async IO is a bad idea which often leads to deadlocks and that is why I don't think that we should allow for switching from sync<=>async even if it's possible using some hacks.

Is there a reason this design was chosen by Windows (decades ago)?

I am sorry but I don't know. I can imagine that it has simplified the design a LOT.

[GSPP]

@adamsitnik Thank you very much! I hadn't considered this approach before, and I didn't find this idea during my research. The internet is silent on the idea that you can reopen a named pipe. But it's only logical since named pipes seem to be implemented ontop of normal file objects. (Thanks for even including source code!)

My first attempt was to reopen the file async just for the WaitForConnect. That would allow for creating a self-contained helper. Unfortunately, that doesn't seem to work (System.IO.IOException: Incorrect function, I assume this is the driver telling us that it doesn't support this operation. Why not? I thought overlapped mode was a property of the handle, but apparently it reaches deeper with pipes). Maybe I'm doing something wrong, but not sure what. I have taken your exact code and then just opened sync switching to async.

I am sorry but I don't know. I can imagine that it has simplified the design a LOT.

I have tried studying the ReactOS code with respect to sync IO and overlapped IO processing. So far I was unable to penetrate it enough to find the core processing functionality of that. I kind of want to see the data structures used to get a clue for why this was done. The original Windows kernel developers are known to be very high-caliber people, so I'm sure they had reasons in mind.

For interested future visitors, I'm linking to the ReactOS (open source Windows clone) source code for the named pipe file system driver: https://doxygen.reactos.org/d2/d9d/drivers_2filesystems_2npfs_2main_8c.html The tree on the left shows all files for that driver. I tried studying that to find a way to break the wait, without success.

The fact that WaitForConnection cannot even be broken by disposing the handle makes this a rather thorny issue. Essentially, most code using that function is exposed to the risk of a permanent hang. I consider this to be a design bug in the named pipe driver.

I also wonder why exiting the process cleans up the pipe but closing the handle does not. At least, that's the way it's supposed to happen. Or maybe, there's a kernel level memory leak or hang of some kind in this case? My understanding is that process termination is implemented by simply closing all handles. A process will remain alive in a zombie state as long as some IO is left. Somehow, Windows seems to handle this.

---

Here is the code for the CancelSynchronousIo solution. Unfortunately, there's a race where the pipe might just have become connected and cancellation will dispose it and cancel some other random IO on the thread... So far, the best I've got is connecting with a dummy client pipe.

internal static void WaitForConnectionCancellable4(NamedPipeServerStream pipeStream, CancellationToken cancellationToken)
{
    if (pipeStream.IsAsync)
        throw new ArgumentException("PipeStream must be synchronous.", nameof(pipeStream));

    //Synchronize so that CancelSynchronousIo is not called after the WaitForConnection operation has completed.

    var threadID = NativeMethods.GetCurrentThreadId();

    //The thread must be opened at a point where it is guaranteed to exist. Can't open it in Cancel.
    using var threadHandle = NativeMethods.OpenThread(NativeMethods.ThreadAccess.TERMINATE, false, threadID);
    if (threadHandle.IsInvalid)
        throw new Win32Exception();

    bool isCompleted = false;
    object lockObject = new();

    void Cancel()
    {
        lock (lockObject)
        {
            if (isCompleted)
                return;

            //There is a potential race condition in that cancellation might happen before WaitForConnection has started.
            //Closing the handle denies further calls to WaitForConnection.
            pipeStream.Dispose();

            NativeMethods.CancelSynchronousIo(threadHandle.DangerousGetHandle()); //Nothing can be done about errors.
        }
    }

    using var cancellationTokenRegistration = cancellationToken.Register(Cancel);

    try
    {
        pipeStream.WaitForConnection();
    }
    finally
    {
        lock (lockObject)
        {
            isCompleted = true;
        }
    }
}

[TestMethod]
public void PipeHelper_WaitForConnectionCancellable4()
{
    foreach (var cancel in new[] { false, true })
    {
        var serverPipeName = "Test-" + Guid.NewGuid().ToString("N");

        using var serverPipe = new NamedPipeServerStream(serverPipeName, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.None);

        var cts = new CancellationTokenSource();

        var waitTask = Task.Run(() => PipeHelper.WaitForConnectionCancellable4(serverPipe, cts.Token));

        if (cancel)
        {
            Thread.Sleep(100); //Give the task time to start
            cts.Cancel();
            waitTask.WhenCompleted().Wait();
            Assert.IsTrue(waitTask.Exception.InnerException is OperationCanceledException);
        }
        else
        {
            using var clientPipe = new NamedPipeClientStream(".", serverPipeName, PipeDirection.InOut, PipeOptions.None);
            clientPipe.Connect(timeout: 0); //This validates that the WaitForConnection call had time to start.
            Assert.IsTrue(clientPipe.IsConnected);
            waitTask.Wait();
            Assert.IsTrue(serverPipe.IsConnected);
        }
    }
}