Search code examples
c#socketsdisposefinalizerxunit

Problem disposing of socket / finalising twice?


I'm working with some code (not mine I hasten to add, I don't trust this much at all) for a class which opens a socket, makes requests and listens for responses, which is throwing an exception in a way I can't comprehend when tested in xunit. I assume the same exception happens "live" but the class is referenced by a singleton so it is probably just hidden.

The problem manifests as "System.CannotUnloadAppDomainException: Error while unloading appdomain" in xunit and the inner exception is "System.ObjectDisposedException" thrown (essentially) inside the finaliser when closing the socket! There are no other reference to the socket which call close and dispose is protected on the Socket class so I'm not clear how else the object could be disposed.

Further, if I merely catch and absorb the ObjectDisposedException xunit terminates when it hits the line to close the listener thread.

I just don't get how the Socket can be disposed before it's asked to close.

My knowledge of sockets is only what I've learnt since finding this problem, so I don't know if I've provided everything SO might need. LMK if not!

public class Foo
{
    private Socket sock = null;
    private Thread tListenerThread = null
    private bool bInitialised;
    private Object InitLock = null;
    private Object DeInitLock = null;

    public Foo()
    {
        bInitialised = false;

        InitLock = new Object();
        DeInitLock = new Object();
    }

    public bool initialise()
    {
        if (null == InitLock)
            return false;

        lock (InitLock)
        {
            if (bInitialised)
                return false;

            sock = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp);
            sock.SetSocketOption(SocketOptionLevel.IP, SocketOptionName.MulticastTimeToLive, 8);
            sock.Bind( /*localIpEndPoint*/);
            sock.SetSocketOption(SocketOptionLevel.IP, SocketOptionName.AddMembership, new MulticastOption(mcIP));

            tListenerThread = new Thread(new ThreadStart(listener));
            tListenerThread.Start();

            bInitialised = true;
            return true;
        }
    }

    ~Foo()
    {
        if (bInitialised)
            deInitialise();
    }

    private void deInitialise()
    {
        if (null == DeInitLock)
            return;

        lock (DeInitLock)
        {
            if (bInitialised)
            {
                sock.Shutdown(SocketShutdown.Both); //throws System.ObjectDisposedException
                sock.Close();

                tListenerThread.Abort(); //terminates xunit test!
                tListenerThread = null;

                sock = null;

                bInitialised = false;
            }
        }
    }
}

Solution

  • If this object is eligible for garbage collection and there are no other references to the Socket, then the socket's finalizer may well run before your object's finalizer. I suspect that's what's happened here.

    It's generally a bad idea (IMO) to do this much work in a finalizer. I can't remember the last time I implemented a finalizer at all - if you implement IDisposable, you should be fine unless you have direct references to unmanaged resources, which are almost always in the form of IntPtrs. Orderly shutdown should be the norm - a finalizer should only usually run if either the program is shutting down, or someone has forgotten to dispose of the instance to start with.

    (I know you clarified at the start that this isn't your code - I just thought I'd explain why it's problematic. Apologies if you already knew some/all of this.)