Managing the disposal of network connections

I am writing a class — let’s call it MessageSender — that needs to perform operations over the network. It basically does these things:

  1. Take some configuration
  2. establish a connection
  3. send stuff

If we ignore the cleanup of any resources, this would look like this:

var sender = new MessageSender("127.0.0.1");
sender.Connect();
sender.SendMessage("Hello world");

The thing I am unsure about is how to manage the disposal of the established connection. I thought of three options, of which I ended up implementing the last one.

(1) Having a dedicated Disconnect() method the user must call:

var sender = new MessageSender("127.0.0.1");
sender.Connect();
sender.SendMessage("Hello world");
sender.Disconnect();

(2) The MessageSender implements IDisposable:

using (var sender = new MessageSender("127.0.0.1"))
{
    sender.Connect();
    sender.SendMessage("Hello world");
}

(3) The Connect() method returns an IDisposable:

var sender = new MessageSender("127.0.0.1");
using (var connection = sender.Connect())
{
    sender.SendMessage("Hello world");
}

I have never seen the third option anywhere, but it does seem to have some advantages:

  • Construction, and hence configuration of the message sender is separated from establishing a connection. E.g. the object itself can be some other classes’ member, constructed and passed down as a dependency to others, while the execution and therefore the actual connection can be deferred to some Run() method.
  • The need for connection tear-down is a direct result from the connection set-up and cannot be (accidentally) separated.
  • If implemented that way, Connect() could be called multiple times.
  • Using IDisposable in general over a dedicated tear-down method gives you better language support, e.g. the using-clauses I used in both (2) and (3).

Potential pitfalls I see for all of the above solutions:

  • Failing to run the tear-down logik. This includes:
    • for (1): Forgetting to call Disconnect()
    • for (2) and (3): Forgetting to propery handle disposables
    • for (3): Ignoring the return value of Connect() entirely
  • MessageSender needs to keep track of its connection state to disallow multiple calls to Connect().
  • Calling SendMessage can fail at runtime depending on the current connection state.

Are there advantages to other approaches or disadvantages to my approach I am not aware of?

Here’s a somewhat simplified version of my actual code:

public sealed class MessageSender
{
    private readonly Some3rdPartyNetworkClient _client;
    private bool _connected = false;

    public MessageSender(string connectionString)
    {
        _client = new Some3rdPartyNetworkClient(connectionString);
    }

    public void SendMessage(string message)
    {
        if (!_connected) throw new InvalidOperationException("not connected.");
        _client.SendMessage(message);
    }

    private sealed class DelegateDisposer : IDisposable
    {
        private readonly Action _dispose;
        public DelegateDisposer(Action dispose) => _dispose = dispose;
        public void Dispose() => _dispose();
    }

    public IDisposable Connect()
    {
        if (_connected) throw new InvalidOperationException("Can only ever connect once.");
        _connected = true;
        
        _client.Connect();
        var tokenSource = new CancellationTokenSource();
        Task checkConnectivityWorker = CheckConnectivityWorker(tokenSource.Token);
        return new DelegateDisposer(() =>
        {
            tokenSource.Cancel();
            if (!checkConnectivityWorker.IsCanceled) checkConnectivityWorker.Wait();
            _client.Disconnect();
        });
    }

    private async Task CheckConnectivityWorker(CancellationToken cancellationToken)
    {
        // some stuff that needs to be done continuously while the connection is active
    }
}

Go to Source
Author: Felk