8

I have an application which uses SslStream to send and receive data with its own fixed-length framing. The stream is created by wrapping the NetworkStream returned from TcpClient.GetStream() like so:

var client = new TcpClient();
client.Connect(host, port);
var sslStream = new SslStream(client.GetStream(), false, callback, null);
sslStream.AuthenticateAsClient(hostname);

Because the protocol is fully asynchronous (framed "messages" arrive at arbitrary times and the client is allowed to send them at arbitrary times), I would normally spawn a thread responsible for blocking on NetworkStream.Read() and otherwise ensure that there is only one thread calling NetworkStream.Write(...) at any one time.

The Remarks section for NetworkStream says:

Read and write operations can be performed simultaneously on an instance of the NetworkStream class without the need for synchronization. As long as there is one unique thread for the write operations and one unique thread for the read operations, there will be no cross-interference between read and write threads and no synchronization is required.

However, the MSDN documentation "Thread Safety" section for SslStream says:

Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

Because SslStream and NetworkStream aren't in the same class hierarchy, I have assumed (perhaps incorrectly) that the remarks for NetworkStream don't apply to SslStream.

Is the best approach for thread safety to simply wrap SslStream.BeginRead / SslStream.EndRead and SslStream.BeginWrite / SslStream.EndWrite with something like this?

internal sealed class StateObject
{
    private readonly ManualResetEvent _done = new ManualResetEvent(false);

    public int BytesRead { get; set; }
    public ManualResetEvent Done { get { return _done; } }
}

internal sealed class SafeSslStream
{
    private readonly object _streamLock = new object();
    private readonly SslStream _stream;

    public SafeSslStream(SslStream stream)
    {
        _stream = stream;
    }

    public int Read(byte[] buffer, int offset, int count)
    {
        var state = new StateObject();
        lock (_streamLock)
        {
             _stream.BeginRead(buffer, offset, count, ReadCallback, state);
        }
        state.Done.WaitOne();
        return state.BytesRead;
    }

    public void Write(byte[] buffer, int offset, int count)
    {
        var state = new StateObject();
        lock (_streamLock)
        {
            _stream.BeginWrite(buffer, offset, count, WriteCallback, state);
        }
        state.Done.WaitOne();
    }

    private void ReadCallback(IAsyncResult ar)
    {
        var state = (StateObject)ar.AsyncState;
        lock (_streamLock)
        {
            state.BytesRead = _stream.EndRead(ar);
        }
        state.Done.Set();
    }

    private void WriteCallback(IAsyncResult ar)
    {
        var state = (StateObject)ar.AsyncState;
        lock (_streamLock)
        {
            _stream.EndWrite(ar);
        }
        state.Done.Set();
    }
}
Hunter Morris
  • 417
  • 3
  • 8
  • Tiny detail: locking on _stream will work but it is advised to create and use a separate object for this purpose. – H H Nov 01 '11 at 17:05
  • @HenkHolterman Cheers for the advice; I updated the code. – Hunter Morris Nov 01 '11 at 17:09
  • FWIW: The [SafeSslStream](https://github.com/smarkets/IronSmarkets/blob/master/IronSmarkets/Sockets/SafeSslStream.cs) in question is for the [IronSmarkets](https://github.com/smarkets/IronSmarkets) project. – Hunter Morris Nov 01 '11 at 17:15

1 Answers1

4

The phrase you pulled from the MSDN docs is a catch-all that is placed in the documentation of most classes. If a member's documentation explicitly states thread safety (as NetworkStream does) then you can rely on it.

However, what they are stating is that you can perform one read and one write at the same time, not two reads or two writes. As such, you will need to synchronise or queue your reads and writes separately. Your code looks sufficient to do this.

Polynomial
  • 27,674
  • 12
  • 80
  • 107
  • What still has me unsure is that the remarks only appear in `NetworkStream` and `SslStream` doesn't itself inherit from `NetworkStream` but instead appears to wrap it when used with `TcpClient`. – Hunter Morris Nov 01 '11 at 17:03
  • Since your SslStream will be wrapping a NetworkStream, you get implicit thread safety on read and write operations independantly. If this weren't the case, SSL would be half-duplex (which it is not). You can just wrap read and write in their own locks and it will be safe. – Polynomial Nov 01 '11 at 17:09
  • 1
    Just to add some precision: while most of SSL is full-duplex, the handshakes require synchronization of the read and write directions; and the client and server may request a re-handshake at any time. Thus, being able to do reads in one thread and writes in the other, with no synchronization, is not an obvious consequence of the thread-safety of NetworkStream. Fortunately, Microsoft's implementation of SslStream includes the necessary internal locks to allow for such bi-thread operation (I haven't checked Mono, though). – Thomas Pornin Feb 10 '16 at 18:55
  • @ThomasPornin Good shout. I'll check Mono shortly to see if they mirror that safety; would make for some interesting bugs if not! – Polynomial Feb 15 '16 at 15:42