c# - Is it safe to call BeginXXX without calling EndXXX if the instance is already disposed

Question

When using the Asynchronous Programming Model it is usually recommended to match every BeginXXX with an EndXXX, otherwise you risk leaking resources until the asynchronous operation completes.

Is that still the case if the class implements IDisposable and the instance was disposed by calling Dispose?

If for example I use UdpClient.BeginReceive in a UdpListener:

class UdpListener : IDisposable
{
    private bool _isDisposed;
    private readonly IPAddress _hostIpAddress;
    private readonly int _port;
    private UdpClient _udpClient;
    public UdpListener(IPAddress hostIpAddress, int port)
    {
        _hostIpAddress = hostIpAddress;
        _port = port;
    }
    public void Start()
    {
        _udpClient.Connect(_hostIpAddress, _port);
        _udpClient.BeginReceive(HandleMessage, null);
    }
    public void Dispose()
    {
        if (_isDisposed)
        {
            throw new ObjectDisposedException("UdpListener");
        }
        ((IDisposable) _udpClient).Dispose();
        _isDisposed = true;
    }
    private static void HandleMessage(IAsyncResult asyncResult)
    {
        // handle...
    }
}

Do I still need to make sure UdpClient.EndReceive is called on the disposed _udpClient (which will just result in an ObjectDisposedException)?

---

Edit:

It isn't uncommon to dispose a UdpClient (and other IDisposables) before all asynchronous operations completed as a way to cancel or implement a timeout, especially over operations that will never complete. This is also what's recommended throughout this site.

Answer 1

When using the Asynchronous Programming Model it is usually recommended to match every BeginXXX with an EndXXX, otherwise you risk leaking resources kept while the asynchronous operation is still "running".

Is that still the case if the class implements IDisposable and Dispose was called on the instance?

This has nothing to do with a class implementing IDisposable or not.

Unless you can be sure that the async completion will free up any resources tied up with the async operation initiated through BeginXXX, and no cleanup is performed in, or as a result of the EndXXX call, you need to ensure that you match your calls. The only way to be certain of this, is to inspect the implementation of a specific async operation.

For the UdpClient example you chose, it happens to be the case that:

1. Calling EndXXX after disposing the UDPClient instance will result in it directly throwing an ObjectDisposedException.
2. No resources are disposed in or as a result of the EndXXX call.
3. The resources tied up with this operation (native overlapped and pinned unmanaged buffers), will be recycled on the async operation completion callback.

So in this case it is perfectly safe, without leakage.

As a general approach

I don't believe this approach is correct as a general approach, because:

1. The implementation could change in the future, breaking your assumptions.
2. There are better ways to do this, using cancellation and time-outs for your async (I/O) operations (e.g. by calling Close on the _udpClient instance to force an I/O failure).

Also, I would not want to rely on me inspecting the entire call stack (and not making a mistake in doing so) to ensure that no resources will be leaked.

Recommended and documented approach

Please note the following from the documentation for the UdpClient.BeginReceive method:

The asynchronous BeginReceive operation must be completed by calling the EndReceive method. Typically, the method is invoked by the requestCallback delegate.

And the following for the underlying Socket.BeginReceive method:

The asynchronous BeginReceive operation must be completed by calling the EndReceive method. Typically, the method is invoked by the callback delegate.

To cancel a pending BeginReceive, call the Close method.

I.e. this is the "by design" documented behavior. You can argue whether the design is very good, but it is clear in what the expected approach to cancellation is, and the behavior that you can expect as the result of doing so.

For your specific example (updated to do something useful with the async result), and other situations similar to it, the following would be an implementation that follows the recommended approach:

public class UdpListener : IDisposable
{
    private readonly IPAddress _hostIpAddress;
    private readonly int _port;
    private readonly Action<UdpReceiveResult> _processor;
    private TaskCompletionSource<bool> _tcs = new TaskCompletionSource<bool>();
    private CancellationTokenSource _tokenSource = new CancellationTokenSource();
    private CancellationTokenRegistration _tokenReg;
    private UdpClient _udpClient;

    public UdpListener(IPAddress hostIpAddress, int port, Action<UdpReceiveResult> processor)
    {
        _hostIpAddress = hostIpAddress;
        _port = port;
        _processor = processor;
    }

    public Task ReceiveAsync()
    {
        // note: there is a race condition here in case of concurrent calls 
        if (_tokenSource != null && _udpClient == null)
        {
            try 
            {
                _udpClient = new UdpClient();
                _udpClient.Connect(_hostIpAddress, _port);
                _tokenReg = _tokenSource.Token.Register(() => _udpClient.Close());
                BeginReceive();
            }
            catch (Exception ex)
            {
                _tcs.SetException(ex);
                throw;
            }
        }
        return _tcs.Task;
    }

    public void Stop()
    {
        var cts = Interlocked.Exchange(ref _tokenSource, null);
        if (cts != null)
        {
            cts.Cancel();
            if (_tcs != null && _udpClient != null)
                _tcs.Task.Wait();
            _tokenReg.Dispose();
            cts.Dispose();
        }
    }

    public void Dispose()
    {
        Stop();
        if (_udpClient != null) 
        {
            ((IDisposable)_udpClient).Dispose();
            _udpClient = null;
        }
        GC.SuppressFinalize(this);
    }

    private void BeginReceive()
    {
        var iar = _udpClient.BeginReceive(HandleMessage, null);
        if (iar.CompletedSynchronously)
            HandleMessage(iar); // if "always" completed sync => stack overflow
    }

    private void HandleMessage(IAsyncResult iar)
    {
        try
        {
            IPEndPoint remoteEP = null;
            Byte[] buffer = _udpClient.EndReceive(iar, ref remoteEP);
            _processor(new UdpReceiveResult(buffer, remoteEP));
            BeginReceive(); // do the next one
        }
        catch (ObjectDisposedException)
        {
            // we were canceled, i.e. completed normally
            _tcs.SetResult(true);
        }
        catch (Exception ex)
        {
            // we failed.
            _tcs.TrySetException(ex); 
        }
    }
}