Destructor never gets called

Question

I have a class Class that creates a Thread in it's constructor. This thread runs a while(true) loop that is reading non-critical data from a NetStream. The thread will be aborted by the destructor:

~Class()
{
 _thread.Abort();
 _thread = null;
}

When the program wants to end the use of Class's instance - ClassInstance, it calls:

ClassInstance = null;
GC.Collect;

I thought that means that ~Class() will be caller automatically at that point - but it's not.

This thread keeps running even after Application.Exit() and returning from Main().

The C# class destructor [is not guaranteed to be called](http://blogs.msdn.com/b/ericlippert/archive/2010/01/21/what-s-the-difference-between-a-destructor-and-a-finalizer.aspx). Use the [Dispose pattern](http://msdn.microsoft.com/en-us/library/fs2xkftw.aspx) for deterministic cleanup of resources. — Dustin Kingen, Aug 28 '13 at 14:01
Instead of calling GC.Collect use the using() statement http://msdn.microsoft.com/en-us/library/yh598w02.aspx The general advise is that you should not call GC.Collect -> http://stackoverflow.com/questions/478167/when-is-it-acceptable-to-call-gc-collect — Oliver, Aug 28 '13 at 14:04
Setting an instance variable to `null` is *not the same* as calling the destructor! Actually it has virtually not effect on the object instance, only one less reference. — Thorsten Dittmar, Aug 28 '13 at 14:14
For it's worth the finalizer actually will run in almost all trivial scenarios. `CriticalFinalizerObject` ties up some of the loose ends, but there are still cases, albeit very limited, where the finalizer may not run to completion. With what information is provided I think it's safe to assume that none of those scenarios are in play here. — Brian Gideon, Aug 28 '13 at 14:39
On another note, calling `Thread.Abort` is a horrible idea. It's like stopping a car by shooting the driver in the head. The car will stop, but there's no telling what damage it's going to do in the process. — Jim Mischel, Aug 28 '13 at 15:16
@JimMischel good note, but this is done only after `_thread` has given the chance to end gracefully , as an `Event` its listening to has signaled , and the terminator has already waited. — Edgar James luffternstat, Nov 26 '13 at 18:14

Brian Gideon · Accepted Answer · 2013-08-28T14:43:06.203

The crucial bit of your code is not included; how the thread is started and what method it is running. If I had to make a guess I would say it is likely you started the thread by passing an instance method of Class. So basically your class instance is still rooted by the running of the thread. You attempt to stop the thread in the finalizer, but the finalizer will never run because the instance is still rooted leading to a catch-22 situation.

Also, you mentioned that the thread is running non-critical code and that was your justification for using Thread.Abort. That really is not a good enough reason. It is very hard to control where that ThreadAbortException will get injected into the thread and as a result it may corrupt critical program data structures you did not anticipate.

Use the new cooperative cancellation mechanisms included with the TPL. Change the while (true) loop to poll a CancellationToken instead. Signal the cancellation in the Dispose method when you implement IDisposable. Do not include a finalizer (destructor in C# terminology). Finalizers are intended to be used to clean up unmanaged resources. Since you have not indicated that unmanaged resources are in play then it is pointless to have a finalizer. You do not have to include a finalizer when implementing IDisposable. In fact, it is considered bad practice to have one when it is not really needed.

public class Class : IDisposable
{
  private Task task;
  private CancellationTokenSource cts = new CancellationTokenSource();

  Class()
  {
    task = new Task(Run, cts.Token, TaskCreationOptions.LongRunning);
    task.Start();
  }

  public void Dispose()
  {
    cts.Cancel();
  }

  private void Run()
  {
    while (!cts.Token.IsCancellationRequested)
    {
      // Your stuff goes here.
    }
  }
}

I recently switched some code in a GUI that used `BackgroundWorker` to use the `TPL` and was very impressed. — SteveB, Aug 28 '13 at 14:31

score 8 · Answer 2 · edited May 23 '17 at 12:07

8

If you implement IDisposable, and dispose the object, then the code in Dispose will run, but there is no guarantee that Destructor will also be called.

Garbage Collector forms an opinion that it is a waste of time. So if you want to have a predictable dispose you can use IDisposable.

Check this Thread

edited May 23 '17 at 12:07

Community

1
1

answered Aug 28 '13 at 14:03

Rahul Tripathi

168,305
31
280
331

Sriram Sakthivel · Answer 3 · 2013-08-28T14:27:12.150

CLR maintains all the running threads. You will have passed the InstanceMethod of your class to the thread's constructor as either ThreadStart or ParameterizedThreadStart delegate. Delegate will hold the MethodInfo of the method you passed and the Instance of your class in Target Property.

Garbage collector collects and object which should not have any Strong References but your instance is still alive inside Delegate of Thread. So your class is still having the Strong Reference hence it is not eligible for garbage collection.

To prove what I stated above

public class Program
{
    [STAThread]
    static void Main(string[] args)
    {
        GcTest();

        Console.Read();
    }

    private static void GcTest()
    {
        Class cls = new Class();
        Thread.Sleep(10);
        cls = null;
        GC.Collect();
        GC.WaitForPendingFinalizers();
    }
}

public class Class
{
    private Thread _thread;
    ~Class()
    {
        Console.WriteLine("~Class");
        _thread.Abort();
        _thread = null;
    }

    public Class()
    {
        _thread = new Thread(ThreadProc);
        _thread.Start();
    }

    private void ThreadProc()
    {
        while (true)
        {
            Thread.Sleep(10);
        }
    }
}

}

Try the above code. Destructor Will not be called. To make it work mark the ThreadProc method as static and run again Destructor will be called

score 2 · Answer 4 · answered Aug 28 '13 at 14:14

Slightly off-topic: You can use Tasks instead of naked threads to run functions without worrying about disposal.

There are multiple issues here:

Setting the variable to null doesn't delete anything, it simply removes a reference to your instance.
The destructor will only get called when the garbage collector decides to collect your instance. The garbage collector runs infrequently, typically only when it detects that there is memory pressure.
The garbage collector collects ONLY orphaned collections. Orphaned means that any references pointed to by your object are invalid.

You should implement the IDisposable interface and call any cleanup code in the Dispose method. C# and VB offer the using keyword to make disposal easier even in the face of exception.

A typical IDisposable implementation is similar to the following:

class MyClass:IDisposable
{
    ClassB _otherClass;

    ...


    ~MyClass()
    {
         //Call Dispose from constructor
         Dispose(false);
    }

    public void Dispose()
    {
        //Call Dispose Explicitly
        Dispose(true);
        //Tell the GC not call our destructor, we already cleaned the object ourselves
        GC.SuppressFinalize(this);
    }

    protected virtual Dispose(bool disposing)
    {
        if (disposing)
        {
            //Clean up MANAGED resources here. These are guaranteed to be INvalid if 
            //Dispose gets called by the constructor

            //Clean this if it is an IDisposable
            _otherClass.Dispose();

           //Make sure to release our reference
            _otherClass=null;
        }
        //Clean UNMANAGED resources here
    }
}

You can then use your class like this :

using(var myClass=new MyClass())
{
    ...
}

Once the using block terminates, Dispose() will be called even if an exception occurs.

Destructor never gets called

4 Answers4

Linked