Memory leak around timer delegate

Question

Sometimes users wants to schedule a large set of timers and don't want to manage the references to those timers.
In case the user does not reference a timer, the timer may be collected by the GC before it was executed.
I created the class Timers to serve as a place holder for newly created timers:

static class Timers
{
    private static readonly ILog _logger = LogManager.GetLogger(typeof(Timers));

    private static readonly ConcurrentDictionary<Object, Timer> _timers = new ConcurrentDictionary<Object, Timer>();

    /// <summary>
    /// Use this class in case you want someone to hold a reference to the timer.
    /// Timer without someone referencing it will be collected by the GC even before execution.
    /// </summary>
    /// <param name="dueTime"></param>
    /// <param name="action"></param>
    internal static void ScheduleOnce(TimeSpan dueTime, Action action)
    {
        if (dueTime <= TimeSpan.Zero)
        {
            throw new ArgumentOutOfRangeException("dueTime", dueTime, "DueTime can only be greater than zero.");
        }
        Object obj = new Object();

        Timer timer = new Timer(state =>
        {
            try
            {
                action();
            }
            catch (Exception ex)
            {
                _logger.ErrorFormat("Exception while executing timer. ex: {0}", ex);
            }
            finally
            {
                Timer removedTimer;
                if (!_timers.TryRemove(obj, out removedTimer))
                {
                    _logger.Error("Failed to remove timer from timers");
                }
                else
                {
                    removedTimer.Dispose();
                }
            }
        });
        if (!_timers.TryAdd(obj, timer))
        {
            _logger.Error("Failed to add timer to timers");
        }
        timer.Change(dueTime, TimeSpan.FromMilliseconds(-1));
    }
}

If I don't dispose the removed timer, it results with a memory leak.
Seems like someone is holding a reference to the Timer's delegate after the timer was removed from the _timers collection.

The question is, why do I get a memory leak if I don't dispose the timer?

Answer 1

The Timer is kept alive by a GCHandle that is created by the timer itself. This can be tested using a .net memory profiler. In turn the Timer will keep the delegate alive, which will then keep the rest alive.

A GCHandle is a special kind of object that can be used to "trick" the Garbage Collector to keep unreachable objects alive.

You could actually kind-of test this without a profiler using:

var a = new ClassA();
var timer = new Timer(a.Exec);

var refA = new WeakReference(a);
var refTimer = new WeakReference(timer);

a = null;
timer = null;

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();

Console.WriteLine(refA.IsAlive);
Console.WriteLine(refTimer.IsAlive);

Answer 2

Timers are Components. As such you must call Dispose when you are done with them.

From the documentation:

A Component should release resources explicitly by calls to its Dispose method, without waiting for automatic memory management through an implicit call to the Finalize method. When a Container is disposed, all components within the Container are also disposed.

The part "When a Container is disposed, all components within the Container are also disposed." can be seen in a Form's Dispose method when it calls:

if (disposing && (components != null))
{
    components.Dispose();
}

So do not expect the timers to be disposed with the Form unless they were added to components.

Update for your comment:
A timer has pointers to unmanaged code (the OS's timer API) so it cannot dispose until those are no longer needed. The finalizer will not run on the object unless dispose is called first or the program is exiting. This is because of these current references to unmanaged code.

From what I understand the dispose model was suppose to speed up programs closing (since the run time could collect the trash during down time) while still allowing the execution of unmanaged code. If you do a large number of ddl imports you will start to see why the system works the way it does.

It should also be noted that the documentation indicates that you may not have access to managed objects from an object's finalizer. An example of this is a StreamWriter. Personally I think this is an arbitrary rule, but it does exist, hence the need for the dispose system.

Either way, if you use something that implements the iDisposable interface, you should always dispose of it when you are done with it. You will get better (more consistent) results that way.