C# Garbage collection of object that has a running timer

Question

I have created an object that needs to update it's status in a set interval so I am using a Timer for this. Many of these objects are displayed in different lists. And when such a list is reloaded(which happens often) the object is no longer referenced, the timer is still running tho.

I know I can prevent this by calling dispose, but a developer using this object can forgot to implement the call to dispose and I think will cause a mayor memory leak, am I right in this and what tools can I employ to make sure the object get garbage collected.

Below is a simplified version of how the classes are used.

The Object

public class Test : IDisposable
{
    protected readonly Timer Timer;
    protected int Count = 0;
    private bool disposed = false;

    public Test()
    {
        Timer = new Timer();
        Timer.Interval = TimeSpan.FromSeconds(1).TotalMilliseconds;

        Timer.Elapsed += Timer_Elapsed;
        Timer.AutoReset = true;
        Timer.Start();
    }

    private void Timer_Elapsed(object sender, ElapsedEventArgs e)
    {
        Count++;
    }

    public void Dispose() // implementing IDisposable
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                // Free other state (managed objects).
            }
            Timer.Stop();
            Timer.Elapsed -= Timer_Elapsed;
            Timer.Dispose();

            // Free your own state (unmanaged objects). 
            // Set large fields to null.
            disposed = true;
        }
    }

    // Use C# destructor syntax for finalization code.
    ~Test()
    {
        // Simply call Dispose(false).
        Dispose(false);
    }
}

The List class

public partial class MainWindow : Window
{

    public List<Test> TestsCollection = new List<Test>();
    public MainWindow()
    {
        InitializeComponent();
    }

    private void Button_Click(object sender, RoutedEventArgs e)
    {
        TestsCollection.Clear();
        TestsCollection.Add(new Test());
        GC.Collect();
    }
}

_"but a developer using this object can forgot to implement the call to dispose"_ should not be a reason. The 'developer' should know how to use the component. Same applies to `Bitmap` and files, for example. What if the 'developer' inherited your `Test` class and overrides the `Dispose` and forgets to call `base.Dispose()` — Jeroen van Langen, Mar 30 '17 at 09:30
@EvK In my test project the Finalizer is never called tho. So the timers keep ticking away happily forever. — KCJ Hendriks, Mar 30 '17 at 09:38
@Hans Passant Well all the code for the test project is posted, I assumed the finalizer was enough hence the test project to test it. But it does not seem to be enough. — KCJ Hendriks, Mar 30 '17 at 09:44
@HansPassant I think in this case destructor is indeed not enough. OP has reference to Timer in a field, and Timer has a reference to object because callback function is not static, so neither of them will be garbage collected and timer will indeed tick forever. — Evk, Mar 30 '17 at 10:04
@HansPassant can you confirm that my answer makes sense (in the part about this setup is not collectible by GC)? I was quite sure when I wrote it but now have some doubts... — Evk, Mar 30 '17 at 15:42
I doubt that [Kelly Johnson](https://en.wikipedia.org/wiki/KISS_principle) would approve of it. — Hans Passant, Mar 30 '17 at 15:50
Does this answer your question? [Can Timers get automatically garbage collected?](https://stackoverflow.com/questions/18136735/can-timers-get-automatically-garbage-collected) — Michael Freidgeim, Nov 20 '20 at 12:33

score 1 · Accepted Answer · edited May 23 '17 at 12:09

In this case it seems there will indeed be a leak if you won't Dispose your Test, and finalizer won't help (at least all my tests show that this is how it will behave).

As stated here - timer internally registers callback in GCHandle table, which prevents situation that callback might be invalid when it's time to fire it. Because your callback (Timer_Elapsed) is not static - that means it holds reference to this and so your Test object cannot be collected, because there is rooted reference (GCHandle) to your callback. Now, your object in turn has reference to timer, because you store it in a field. So after all, there is reference from root to both your Test object and your timer, and neither of them can be garbage collected.

To fix the issue - you need to break some of the strong references. You can do that for example like this:

public class Test : IDisposable {
    protected readonly Timer Timer;
    protected int Count = 0;
    private bool disposed = false;

    public Test() {
        Timer = new Timer(Timer_Elapsed, new WeakReference<Test>(this), 0, 1000);
    }

    private static void Timer_Elapsed(object state) {
        var wr = (WeakReference<Test>) state;
        Test test;
        if (wr.TryGetTarget(out test)) {
            test.Count++;
            Console.WriteLine(test.Count);
        }
    }

    public void Dispose() // implementing IDisposable
    {
        if (!disposed) {
            Timer.Dispose();
            disposed = true;
        }
    }
}

There we first replace System.Timers.Timer with System.Threading.Timer and also make callback method static, so that it no longer prevent parent Test object from collection. However, we need to access Test object somehow in callback - so we pass WeakReference as a state there (we cannot pass object itself because it will create the same situation we are trying to avoid). In callback we check if Test object is still alive, and if yes - do what is needed with it.

We also remove destructor because it's not necessary here. When Test object is garbage collected - Timer will also be collected and it's own destructor will be called.

C# Garbage collection of object that has a running timer

1 Answers1