Improve NamedPipeClientStream.Connect edge cases handling

[adamsitnik]

Based on my understanding of the two following quotes from the docs:

https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew

If the CreateNamedPipe function was not successfully called on the server prior to this operation, a pipe will not exist and CreateFile will fail with ERROR_FILE_NOT_FOUND

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

If no instances of the specified named pipe exist, the WaitNamedPipe function returns immediately, regardless of the time-out value.

There is no point of calling WaitNamedPipe right after CreateFileW failed with ERROR_FILE_NOT_FOUND (we are going to get the same error, unless the pipe was created in the meantime). Imo it's better to return false and let the re-try logic retry in a moment.

The C++ example also calls WaitNamedPipe only when CreateFileW failed with ERROR_PIPE_BUSY.

Based on

A subsequent CreateFile call to the pipe can fail, because the instance was closed by the server or opened by another client.

I've extended the condition to not fail on ERROR_FILE_NOT_FOUND as suggested by @vonzshik in #65434

fixes #65434

[ghost]

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

Issue Details

---

Based on my understanding of the two following quotes from the docs:

https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew

If the CreateNamedPipe function was not successfully called on the server prior to this operation, a pipe will not exist and CreateFile will fail with ERROR_FILE_NOT_FOUND

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

If no instances of the specified named pipe exist, the WaitNamedPipe function returns immediately, regardless of the time-out value.

There is no point of calling WaitNamedPipe right after CreateFileW failed with ERROR_FILE_NOT_FOUND (we are going to get the same error, unless the pipe was created in the meantime). Imo it's better to return false and let the re-try logic retry in a moment.

The C++ example also calls WaitNamedPipe only when CreateFileW failed with ERROR_PIPE_BUSY.

Based on

A subsequent CreateFile call to the pipe can fail, because the instance was closed by the server or opened by another client.

I've extended the condition to not fail on ERROR_FILE_NOT_FOUND as suggested by @vonzshik in #65434

fixes #65434

Author:	adamsitnik
Assignees:	-
Labels:	area-System.IO
Milestone:	7.0.0

[adamsitnik]

FWIW I've tried to create a nice unit test based on repro provided in #65434 but I've failed (it would not be reliable). So I've used repro provided by @vonzshik to test the fix.

Here is what I had:

static async Task Repro(CancellationToken cancellationToken)
{
    const int ClientCount = 10;
    const int ServerCount = 3;

    int timeout = (int)TimeSpan.FromSeconds(0.03).TotalMilliseconds;
    Barrier barrier = new Barrier(ClientCount + ServerCount);

    var clientTasks = Enumerable.Range(0, ClientCount).Select(i => Task.Run(async () =>
    {
        await using var client = new NamedPipeClientStream("foo");

        try
        {
            Task connect = client.ConnectAsync(timeout, cancellationToken);

            while (connect.Status != TaskStatus.Running) ;

            barrier.SignalAndWait();

            await connect;
        }
        catch (TimeoutException)
        {
            return;
        }

        await client.WriteAsync(new byte[1], cancellationToken);
    }, cancellationToken)).ToArray();

    var serverTasks = Enumerable.Range(0, ServerCount).Select(i => Task.Run(async () =>
    {
        barrier.SignalAndWait(); // wait until clients start to connect

        await using var server = new NamedPipeServerStream("foo", PipeDirection.InOut, ServerCount);
        await server.WaitForConnectionAsync(cancellationToken);
        await server.ReadAsync(new byte[1], cancellationToken);
    }, cancellationToken)).ToArray();

    await Task.WhenAll(clientTasks);
    await Task.WhenAll(serverTasks);
}

[adamsitnik]

@stephentoub @jozkee PTAL

[stephentoub]

Thanks. Test?

[adamsitnik]

Test?

#65553 (comment)