Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is CancellationTokenSource.CancelAfter() leaky?

The release of the Async Targeting Pack prompted me to use ILSpy to have a look at what Task-based Asynchronous Pattern (TAP) extension methods were provided there (some of which I have already implemented on my own for use in VS2010). I stumbled upon the .CancelAfter(TimeSpan) method for CancellationTokenSource (which is as an extension method in the Async Targeting Pack for .NET 4.0 but is an instance method in .NET 4.5) and thought it could be a nice way to implement a timeout for various operations that don't natively have a timeout, but do support cancellation.

But looking at the implementation in the Async Targeting Pack, it seems that if the associated Task completes or is canceled, the timer keeps on running.

/// <summary>Cancels the <see cref="T:System.Threading.CancellationTokenSource" /> after the specified duration.</summary>
/// <param name="source">The CancellationTokenSource.</param>
/// <param name="dueTime">The due time in milliseconds for the source to be canceled.</param>
public static void CancelAfter(this CancellationTokenSource source, int dueTime)
{
    if (source == null)
    {
        throw new NullReferenceException();
    }
    if (dueTime < -1)
    {
        throw new ArgumentOutOfRangeException("dueTime");
    }
    Timer timer = new Timer(delegate(object self)
    {
        ((IDisposable)self).Dispose();
        try
        {
            source.Cancel();
        }
        catch (ObjectDisposedException)
        {
        }
    });
    timer.Change(dueTime, -1);
}

Let's say I use this method to provide a timeout for a frequently used TAP-based operation, and wrap it with a .CancelAfter(). Now let's say the user provides a timeout value of 5 minutes (300 seconds), and calls this operation 100 times a second, which all complete successfully after a few milliseconds. After 300 seconds of 100 calls per second, 30,000 running timers would have accumulated from all those operations, even though the tasks completed successfully long ago. They would all eventually elapse and run the above delegate, which will probably throw an ObjectDisposedException, etc.

Isn't this somewhat of a leaky, non-scalable behavior? When I implemented a timeout, I used Task/TaskEx.Delay(TimeSpan, CancellationToken) and when the associated task has ended, I cancel the .Delay() so that the timer would be stopped and disposed (it is an IDisposable after all, and it does contain unmanaged resources). Is this cleanup overly zealous? Is the cost of having tens of thousands of timers running simultaneously (and possibly throwing tens of thousands of caught exceptions later) really inconsequential to performance for the average application? Is the overhead and leakiness of .CancelAfter() almost always minuscule compared to the actual work being done, and should generally be disregarded?

like image 878
Allon Guralnek Avatar asked May 23 '12 07:05

Allon Guralnek


1 Answers

Just try it, push it to the limits and see what happens. I can't get working set to go over 90 MB with ten million timers. System.Threading.Timer is very cheap.

using System;
using System.Threading;

class Program {
    public static int CancelCount;
    static void Main(string[] args) {
        int count = 1000 * 1000 * 10;
        for (int ix = 0; ix < count; ++ix) {
            var token = new CancellationTokenSource();
            token.CancelAfter(500);
        }
        while (CancelCount < count) {
            Thread.Sleep(100);
            Console.WriteLine(CancelCount);
        }
        Console.WriteLine("done");
        Console.ReadLine();
    }
}

static class Extensions {
    public static void CancelAfter(this CancellationTokenSource source, int dueTime) {
        Timer timer = new Timer(delegate(object self) {
            Interlocked.Increment(ref Program.CancelCount);
            ((IDisposable)self).Dispose();
            try {
                source.Cancel();
            }
            catch (ObjectDisposedException) {
            }
        });
        timer.Change(dueTime, -1);
    }
}
like image 177
Hans Passant Avatar answered Nov 02 '22 08:11

Hans Passant