53

I am trying to create a function that takes in an Action and a Timeout, and executes the Action after the Timeout. The function is to be non-blocking. The function must be thread safe. I also really, really want to avoid Thread.Sleep().

So far, the best I can do is this:

long currentKey = 0;
ConcurrentDictionary<long, Timer> timers = new ConcurrentDictionary<long, Timer>();

protected void Execute(Action action, int timeout_ms)
{
    long currentKey = Interlocked.Increment(ref currentKey);
    Timer t = new Timer(
      (key) =>
         {
           action();
           Timer lTimer;
           if(timers.TryRemove((long)key, out lTimer))
           {
               lTimer.Dispose();
           }
         }, currentKey, Timeout.Infinite, Timeout.Infinite
      );

     timers[currentKey] = t;
     t.Change(timeout_ms, Timeout.Infinite);
}

The problem is that calling Dispose() from the callback itself cannot be good. I am unsure if it is safe to "fall off" the end, i.e. Timers are considered live while their lambdas are executing, but even if this is the case I'd rather dispose it properly.

The "fire once with a delay" seems like such a common problem that there should be an easy way to do this, probably some other library in System.Threading I am missing, but right now the only solution I can think of is modification of the above with a dedicated cleanup task running on an interval. Any advice?

Chuu
  • 4,301
  • 2
  • 28
  • 54
  • 1
    I kind of fail to see why you would not solve this with the simplest solution, which is Thread.Sleep? – jeroenh May 05 '11 at 22:10
  • Can you explain why do you really, really want to avoid Thread.Sleep()? – Andrew Savinykh May 05 '11 at 22:12
  • 1
    To anyone using this code -- beware! There is a bug in the Timer API that can cause an unmanaged memory leak if the timer is too fine grained (i.e. the Dispose() is called too soon after the .Change() method). As of this comment .NET 4.0 is the latest version, it will hopefully be fixed in a future version. – Chuu Jul 24 '12 at 21:26
  • 7
    @jeroenh: Using `Thread.Sleep` could work if you had one or two actions. But what if you have 100 actions? You really want to create 100 threads that do nothing but sleep? That's 100 megabytes of stack space (among other resources) wasted. – Jim Mischel Apr 18 '13 at 15:22
  • 1
    @JimMischel I have learned since :-); thanks for pointing it out – jeroenh Apr 18 '13 at 19:51

12 Answers12

86

I don't know which version of C# you are using. But I think you could accomplish this by using the Task library. It would then look something like that.

public class PauseAndExecuter
{
    public async Task Execute(Action action, int timeoutInMilliseconds)
    {
        await Task.Delay(timeoutInMilliseconds);
        action();
    }
}
treze
  • 3,159
  • 20
  • 21
  • Well, TPL and Async. Definitely the way to go if you have .NET 4.5. – Jim Mischel Apr 18 '13 at 18:45
  • 1
    My question was submitted pre-4.5, but this solution is definitely the best one assuming Task.Delay can't exhaust some critical resource like sleeping on the Threadpool does. I am still on .NET 4.0, can someone more familiar with 4.5 confirm that it doesn't have the same problem as a lot of the other threadpool/sleep solutions? – Chuu Apr 19 '13 at 09:06
  • http://msdn.microsoft.com/en-us/library/vstudio/hh156528.aspx // The following method runs asynchronously. The UI thread is not // blocked during the delay. You can move or resize the Form1 window // while Task.Delay is running. public async Task WaitAsynchronouslyAsync() { await Task.Delay(10000); return "Finished"; } Maybe that helps – treze Apr 19 '13 at 11:16
  • According to [this question](http://stackoverflow.com/questions/17378548/what-it-costs-to-use-task-delay) Task.Delay uses Timers internally, so I feel it's safe to make this as the correct answer. – Chuu Jan 28 '14 at 16:36
  • 1
    Geeze - that's about as simple as you can get! – jklemmack Apr 15 '14 at 15:43
  • Believe return type should be Task, otherwise void cannot be awaited. – Geoffrey Hudik Sep 23 '15 at 20:59
  • Thank you so much. I love this way. *work with .net 4.5.2 – Cluster Apr 20 '16 at 09:12
  • The "point" of using `async void` is that it does not - in fact, cannot - be waited upon. – user2864740 Jan 16 '18 at 20:01
  • 2
    you can actually make it even simple with ```Task.Delay(timeoutInMilliseconds).ContinueWith(t => yourAction());``` – Do-do-new May 10 '19 at 14:37
31

There is nothing built-in to .Net 4 to do this nicely. Thread.Sleep or even AutoResetEvent.WaitOne(timeout) are not good - they will tie up thread pool resources, I have been burned trying this!

The lightest weight solution is to use a timer - particularly if you will have many tasks to throw at it.

First make a simple scheduled task class:

class ScheduledTask
{
    internal readonly Action Action;
    internal System.Timers.Timer Timer;
    internal EventHandler TaskComplete;

    public ScheduledTask(Action action, int timeoutMs)
    {
        Action = action;
        Timer = new System.Timers.Timer() { Interval = timeoutMs };
        Timer.Elapsed += TimerElapsed;            
    }

    private void TimerElapsed(object sender, System.Timers.ElapsedEventArgs e)
    {
        Timer.Stop();
        Timer.Elapsed -= TimerElapsed;
        Timer = null;

        Action();
        TaskComplete(this, EventArgs.Empty);
    }
}

Then, create a scheduler class - again, very simple:

class Scheduler
{        
    private readonly ConcurrentDictionary<Action, ScheduledTask> _scheduledTasks = new ConcurrentDictionary<Action, ScheduledTask>();

    public void Execute(Action action, int timeoutMs)
    {
        var task = new ScheduledTask(action, timeoutMs);
        task.TaskComplete += RemoveTask;
        _scheduledTasks.TryAdd(action, task);
        task.Timer.Start();
    }

    private void RemoveTask(object sender, EventArgs e)
    {
        var task = (ScheduledTask) sender;
        task.TaskComplete -= RemoveTask;
        ScheduledTask deleted;
        _scheduledTasks.TryRemove(task.Action, out deleted);
    }
}

It can be called as follows - and is very lightweight:

var scheduler = new Scheduler();

scheduler.Execute(() => MessageBox.Show("hi1"), 1000);
scheduler.Execute(() => MessageBox.Show("hi2"), 2000);
scheduler.Execute(() => MessageBox.Show("hi3"), 3000);
scheduler.Execute(() => MessageBox.Show("hi4"), 4000);
Eric Boumendil
  • 2,318
  • 1
  • 27
  • 32
James Harcourt
  • 6,017
  • 4
  • 22
  • 42
6

I use this method to schedule a task for a specific time:

public void ScheduleExecute(Action action, DateTime ExecutionTime)
{
    Task WaitTask = Task.Delay(ExecutionTime.Subtract(DateTime.Now));
    WaitTask.ContinueWith(() => action());
    WaitTask.Start();
}

It should be noted that this only works for about 24 days out because of int32 max value.

VoteCoffee
  • 4,692
  • 1
  • 41
  • 44
5

My example:

void startTimerOnce()
{
   Timer tmrOnce = new Timer();
   tmrOnce.Tick += tmrOnce_Tick;
   tmrOnce.Interval = 2000;
   tmrOnce.Start();
}

void tmrOnce_Tick(object sender, EventArgs e)
{
   //...
   ((Timer)sender).Dispose();
}
Mykola Khyliuk
  • 1,234
  • 1
  • 9
  • 16
3

This seems to work for me. It allows me to invoke _connection.Start() after a 15 second delay. The -1 millisecond parameter just says don't repeat.

// Instance or static holder that won't get garbage collected (thanks chuu)
System.Threading.Timer t;

// Then when you need to delay something
var t = new System.Threading.Timer(o =>
            {
                _connection.Start(); 
            },
            null,
            TimeSpan.FromSeconds(15),
            TimeSpan.FromMilliseconds(-1));
naskew
  • 2,091
  • 1
  • 19
  • 20
  • 1
    If t goes out of scope, the Timer becomes eligible for garbage collection. If collected, the timer will not fire. – Chuu Oct 05 '17 at 18:35
  • You are absolutely right and I should have made it clear that where I was using it I declared it with t being at class level. I'll edit the reply to try and make that clear. – naskew Oct 06 '17 at 08:45
  • Recommendation: swap 'TimeSpan.FromMilliseconds(-1)' for 'Timeout.Infinite'. – tkpb Jul 22 '18 at 04:33
2

The model you have, using a one-shot timer, is definitely the way to go. You certainly don't want to create a new thread for every one of them. You could have a single thread and a priority queue of actions keyed on time, but that's needless complexity.

Calling Dispose in the callback probably isn't a good idea, although I'd be tempted to give it a try. I seem to recall doing this in the past, and it worked okay. But it's kind of a wonky thing to do, I'll admit.

You can just remove the timer from the collection and not dispose it. With no references to the object, it will be eligible for garbage collection, meaning that the Dispose method will be called by the finalizer. Just not as timely as you might like. But it shouldn't be a problem. You're just leaking a handle for a brief period. As long as you don't have thousands of these things sitting around un-disposed for a long period, it's not going to be a problem.

Another option is to have a queue of timers that remains allocated, but deactivated (i.e. their timeout and intervals set to Timeout.Infinite). When you need a timer, you pull one from the queue, set it, and add it to your collection. When the timeout expires, you clear the timer and put it back on the queue. You can grow the queue dynamically if you have to, and you could even groom it from time to time.

That'll prevent you from leaking one timer for every event. Instead, you'll have a pool of timers (much like the Thread Pool, no?).

Jim Mischel
  • 131,090
  • 20
  • 188
  • 351
2

Use Microsoft's Reactive Framework (NuGet "System.Reactive") and then you can do this:

protected void Execute(Action action, int timeout_ms)
{
    Scheduler.Default.Schedule(TimeSpan.FromMilliseconds(timeout_ms), action);
}
Enigmativity
  • 113,464
  • 11
  • 89
  • 172
2

If you don't care much about the granularity of time, you can create one timer that ticks every second and checks for expired Actions that need to be queued on the ThreadPool. Just use the stopwatch class to check for timeout.

You can use your current approach, except your Dictionary will have Stopwatch as its Key and Action as its Value. Then you just iterate on all the KeyValuePairs and find the Stopwatch that expires, queue the Action, then remove it. You'll get better performance and memory usage from a LinkedList however (since you'll be enumerating the whole thing every time and removing an item is easier).

Garo Yeriazarian
  • 2,533
  • 18
  • 29
  • 1
    This is very similar to how the [CreateTimerQueue](http://msdn.microsoft.com/en-us/library/ms686796%28v=VS.85%29.aspx) api works, which is what System.Threading.Timers wraps. – Chuu Jun 16 '11 at 22:29
  • The net effect is the same, except you only have one timer handle to juggle. – Garo Yeriazarian Jun 17 '11 at 13:30
1

treze's code is working just fine. This might help the ones who have to use older .NET versions:

private static volatile List<System.Threading.Timer> _timers = new List<System.Threading.Timer>();
private static object lockobj = new object();
public static void SetTimeout(Action action, int delayInMilliseconds)
{
    System.Threading.Timer timer = null;
    var cb = new System.Threading.TimerCallback((state) =>
    {
        lock (lockobj)
            _timers.Remove(timer);
        timer.Dispose();
        action();
    });
    lock (lockobj)
        _timers.Add(timer = new System.Threading.Timer(cb, null, delayInMilliseconds, System.Threading.Timeout.Infinite));
}
Koray
  • 1,768
  • 1
  • 27
  • 37
1

Documentation clearly states that System.Timers.Timer has AutoReset property made just for what you are asking:

https://msdn.microsoft.com/en-us/library/system.timers.timer.autoreset(v=vs.110).aspx

Lu4
  • 14,873
  • 15
  • 79
  • 132
  • 2
    The problem here is lifetime management, not the timer itself. Specifically, you have to hold a reference to the Timer until it fires or it will be disabled when if the gc collects it; and need to release the reference after it fires so the gc can collect it. This is assuming you are managing the lifetime yourself, which you don't have to with the 'async' solution. – Chuu Dec 09 '16 at 17:11
0

Why not simply invoke your action parameter itself in an asynchronous action?

Action timeoutMethod = () =>
  {
       Thread.Sleep(timeout_ms);
       action();
  };

timeoutMethod.BeginInvoke();
Tejs
  • 40,736
  • 10
  • 68
  • 86
  • BeginInvoke() uses the thread pool, so it has similar problems to other proposed solutions. – Chuu Aug 24 '12 at 15:38
-2

This may be a little bit late, but here is the solution I am currently using to handle delayed execution:

public class OneShotTimer
{

    private volatile readonly Action _callback;
    private OneShotTimer(Action callback, long msTime)
    {
        _callback = callback;
        var timer = new Threading.Timer(TimerProc);
        timer.Change(msTime, Threading.Timeout.Infinite);
    }

    private void TimerProc(object state)
    {
        try {
            // The state object is the Timer object. 
            ((Threading.Timer)state).Dispose();
            _callback.Invoke();
        } catch (Exception ex) {
            // Handle unhandled exceptions
        }
    }

    public static OneShotTimer Start(Action callback, TimeSpan time)
    {
        return new OneShotTimer(callback, Convert.ToInt64(time.TotalMilliseconds));
    }
    public static OneShotTimer Start(Action callback, long msTime)
    {
        return new OneShotTimer(callback, msTime);
    }
}

You can use it like this:

OneShotTimer.Start(() => DoStuff(), TimeSpan.FromSeconds(1))
Karsten
  • 1,814
  • 2
  • 17
  • 32
  • Notes: 1) avoid `dynamic` in non-dynamic situations (like this), 2) the question is about *object lifetime*, which this does not solve. In fact, it's *less likely to work reliably* since the `timer` instance is eligible for reclamation *immediately*. – user2864740 Jan 16 '18 at 19:25
  • @user2864740 1) You are correct. It was translated poorly from VB. – Karsten Feb 05 '18 at 14:30
  • @user2864740 2) Can you elaborate on this? Is a non-disposed instance of the timer object necessary after the callback fired, or do you mean the constructor? I don't think the timer can be garbage collected because it has to be referenced from somewhere to eventually execute the callback. – Karsten Feb 05 '18 at 14:51
  • @user2864740 ...and pass the timer as the state object to the callback. – Karsten Feb 05 '18 at 15:03
  • So where is this 'state' set? It's *not* done in the given code, hence a serious semantic error. Also, I'm not entirely sure passing the timer itself via the state will *ensure* a Strongly-rooted Reference: but that is orthogonal to the issue until the proposed code utilizes the state to attempt to maintain a Strong Reference. – user2864740 Feb 05 '18 at 21:36
  • Wrt "Strongly-rooted Reference": if the Timer is the *only* direct reference to the Timer State, then the Timer State *cannot* keep the Timer alive. – user2864740 Feb 05 '18 at 21:40
  • @user2864740 This is not really true. Timers are implemented with a global [timer queue](https://referencesource.microsoft.com/#mscorlib/system/threading/timer.cs,208ff87939c84fe3) which actually keeps strongly rooted references to timers and their state object. So it will work properly. I just checked that you can garbage collect all you want and the callback will always be called without any problems. The only complaint you may have is that it is not guaranteed in the documentation. – Karsten Feb 05 '18 at 22:28
  • @Karsten I am curious how you checked. I've run into several bugs before with prematurely disposed Timers and looking at the source it appears to behave how I expect -- explicitly removes itself from the Windows timer queue on destruction. Unless this behaviour has changed in recent version of .NET. – Chuu Jul 23 '18 at 14:08