Gracefully closing a named pipe and disposing of streams

Question

I have a two-way named pipe. I'm not sure how to shut it down gracefully, though, once I'm done with it - if I close the connection from the client side, the server side throws an exception when it tries to dispose of the StreamReader and StreamWriter I'm using. I'm currently catching it, but that seems like a kludge job to me.

Server side code:

Thread pipeServer = new Thread(ServerThread);
pipeServer.Start();

private void ServerThread(object data)
{
    int threadId = Thread.CurrentThread.ManagedThreadId;
    log.Debug("Spawned thread " + threadId);

    PipeSecurity ps = new PipeSecurity();
    SecurityIdentifier sid = new SecurityIdentifier(WellKnownSidType.WorldSid, null);
    ps.AddAccessRule(new PipeAccessRule(sid, PipeAccessRights.ReadWrite, System.Security.AccessControl.AccessControlType.Allow));
    ps.AddAccessRule(new PipeAccessRule(WindowsIdentity.GetCurrent().Owner, PipeAccessRights.FullControl, System.Security.AccessControl.AccessControlType.Allow));
    log.Debug("Pipe security settings set [Thread " + threadId + "]");

    NamedPipeServerStream pipeServer =
        new NamedPipeServerStream("RDPCommunicationPipe", PipeDirection.InOut, numThreads, PipeTransmissionMode.Message, PipeOptions.None, 0x1000, 0x1000, ps);

    log.Debug("Pipe Servers created");

    // Wait for a client to connect
    log.Info("Pipe created on thread " + threadId + ". Listening for client connection.");
    pipeServer.WaitForConnection();
    log.Debug("Pipe server connection established [Thread " + threadId + "]");

    Thread nextServer = new Thread(ServerThread);
    nextServer.Start();

    try
    {
        // Read the request from the client. Once the client has
        // written to the pipe its security token will be available.

        using (StreamReader sr = new StreamReader(pipeServer))
        {
            using (StreamWriter sw = new StreamWriter(pipeServer) { AutoFlush = true })
            {
                // Verify our identity to the connected client using a
                // string that the client anticipates.

                sw.WriteLine("I am the one true server!");

                log.Debug("[Thread " + threadId + "]" + sr.ReadLine());

                log.Info(string.Format("Client connected on thread {0}. Client ID: {1}", threadId, pipeServer.GetImpersonationUserName()));
                while (!sr.EndOfStream)
                {
                    log.Debug("[Thread " + threadId + "]" + sr.ReadLine());
                }
            }
        }
    }
    // Catch the IOException that is raised if the pipe is broken
    // or disconnected.
    catch (IOException e)
    {
        log.Error("ERROR: " + e);
    }
}

Client side code:

class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("Starting...");
        var client = new NamedPipeClientStream(".", "RDPCommunicationPipe", PipeDirection.InOut);
        client.Connect();
        Console.WriteLine("Pipe connected successfully");

        using (StreamReader sr = new StreamReader(client))
        {
            using (StreamWriter sw = new StreamWriter(client) { AutoFlush = true })
            {
                string temp;
                do
                {
                    temp = sr.ReadLine();
                    Console.WriteLine(temp);
                } while (temp.Trim() != "I am the one true server!");

                sw.WriteLine("Message received and understood");
                while (!string.IsNullOrEmpty(temp = Console.ReadLine()))
                {
                    sw.WriteLine(temp);
                }
            }
        }
        client.Close();
    }
}

It works perfectly until I hit enter on an empty line in the client app, which terminates it, closing the client. The server app then throws a System.IO.IOException: Pipe is broken. when it hits the end of the StreamWriter using block. How do I properly dispose of my stream handlers?

(Code based on ideas found here and here.)

Answer 1

I'm currently catching it, but that seems like a kludge job to me.

IMHO, it's about as good as you're going to get, if you want to be a good neighbor and dispose your owned StreamWriter object and still invest a minimum of effort.

That said, it seems to me that in this particular situation, it'd also be fine to comment out the call to Dispose() — or in your case, not use the using statement — and include another comment explaining that at that point in the sequence of execution of your code, you know that all that call is going to do is throw an exception, and so there's no point in making it.

Of course, if you just don't bother disposing the StreamWriter, then you'll want to explicitly dispose your pipe stream. You might also want to use the StreamWriter constructor that has the leaveOpen parameter, and pass true for that parameter, as a way of documenting your intent to not have the StreamWriter own the pipe stream object.

Either way, you're going to wind up leaving object in the finalizer queue, because the exception bypasses the call to GC.SuppressFinalize(), as does (of course) not bothering to call Dispose() at all. As long as you aren't dealing with a high-volume scenario (i.e. lots of these objects), that's probably fine. But it's certainly not ideal.

Unfortunately, named pipes themselves don't have semantics that provide for the kind of "graceful closure" that sockets do. That is, the only way for an endpoint to indicate they are done writing is to disconnect (for a server pipe) or close (for server or client pipes). Neither option leaves the pipe available for reading, so implementing a graceful closure on a pipe requires handshaking within the application protocol itself, rather than relying on the I/O object .

In addition to this inconvenience (which I admit, isn't really directly related to your question), the implementation of PipeStream.Flush() checks to see whether the pipe is writeable. Even though it has no intention of writing anything! It's that last part I find really annoying, and of course directly causes the issue you're asking about. It seems unreasonable to me for code in the .NET Framework to go out of its way to throw exceptions in scenarios where those exceptions cause more trouble than good.

All that said, you do have some other options:

1. Subclass the NamedPipeServerStream and NamedPipeClientStream types, and override the Flush() method so that it really does do nothing. Or rather, it would be nice if you could do this. But those types are sealed, so you can't.
2. Alternative to subclassing those types, you can wrap them in your own Stream implementation. This is a lot more of a hassle, especially since you are likely going to want to override all of the async members, at least if you intend to use these objects in any situation where I/O performance is of any interest.
3. Use separate one-directional pipes for reading and writing. In this implementation, you can close the StreamWriter itself as a way of closing the connection, which results in the correct order of things (i.e. the flush happens before the close on the pipe). This also addresses the graceful closure issue, because with two pipes for each connection, you can have the same basic "half-closed" semantics that sockets have. Of course, this option is significantly complicated by the challenge in identifying which pairs of pipe connections go with each other.

Both of these (that is, the second and third ones, i.e. the ones that are actually possible) have some obvious drawbacks. Having to have your own Stream class is a pain, due to all the duplication of code required. And doubling the pipe object count seems like a drastic way to address the exception (but it could be an acceptable and desirable implementation to support the graceful closure semantics, with the happy side-effect of eliminating the thrown-exception issue with StreamWriter.Dispose()).

Note that in a high-volume scenario (but then, why are you using pipes? ), throwing and catching exceptions with high frequency could be a problem (they are expensive). So one or the other of these two alternative options would probably be preferable in that case, over either catching the exception and just not bothering to close/dispose your StreamWriter (both of which add inefficiencies that would interfere with a high-volume scenario).