Calling Method only once from a Threading.Timer

Question

I have a System.Threading.Timer which fires frequently (let's say every second for simplicity), in the CallBack I need to call an Action (which is passed in via the constructor, so sits in another class) within which I do some processing (let's say it takes 2+ seconds), how would I prevent my processing logic from being called multiple times? It seems that a lock() doesn't work within the Action call? I am using .net 3.5.

public TestOperation(Action callBackMethod)
{
    this.timer = new System.Threading.Timer(timer_Elapsed, callbackMethod, timerInterval, Timeout.Infinite);
}

private void timer_Elapsed(object state)
{
    Action callback = (Action) state;
    if (callback != null)
    {
        callback();
    }
}

// example of the callback, in another class. 
private void callBackMethod()
{
    // How can I stop this from running every 1 second? Lock() doesn't seem to work here
    Thread.Sleep(2000);
}

Thanks!

Use a task and let the task schedule a new task when it is done. You can delay this second task a second if needed. — Brian Rasmussen, Apr 22 '14 at 21:08
Create a one-shot timer and update it in the callback. See http://stackoverflow.com/a/684452/56778 — Jim Mischel, Apr 22 '14 at 22:30

spender · Answer 1 · 2014-04-22T21:24:59.117

~~You could do something like this and avoid timers altogether.~~

void Main()
{
    RunPeriodicAsync();
}
async Task RunPeriodicAsync()
{
    while(true)
    {
        await Task.Delay(someTimeSpan);
        DoTheThing();
        if(!goAgain)break;
    }

}

or if you need to support cancellation:

void Main()
{
    var cts=new CancellationTokenSource();
    RunPeriodicAsync(cts.Token);
    //sometime later
    cts.Cancel();
}
async Task RunPeriodicAsync(CancellationToken ct)
{
    while(!ct.IsCancellationRequested)
    {
        await Task.Delay(1000);
        DoTheWork();
    }
}

Without async/await you could:

System.Threading.Timer timer;
void Main()
{
    RunActionAfter(() => DoTheWork(), 2000);
}
void RunActionAfter(Action action, int period)
{
    //Timeout.Infinite means the timer runs only once.
    timer = new Timer(_ => action(), null, 2000, Timeout.Infinite); 
}
void DoTheWork()
{
    Console.WriteLine("!!!");

    //then maybe
    RunActionAfter(() => DoTheWork(),2000);
}

I am running .net 3.5 unfortunately... otherwise this would be a lot simpler. — Richard, Apr 22 '14 at 21:17

score 5 · Accepted Answer · answered Apr 22 '14 at 21:22

5

There's nothing pretty about having to solve this problem. Note that using lock is a very bad idea, it will make your threadpool explode when the callback consistently takes too much time. This happens easily when the machine gets loaded. Using Monitor.TryEnter() is the safe alternative. Definitely not pretty either, you'll arbitrarily lose callbacks.

It gets a heckofalot easier if you simply set the period argument to 0. So that the timer can tick only once. Now you automatically have a hard guarantee that the callback cannot be re-entered. All you have to do is call Change() at the end of the method to restart the timer. It is up to you to use a fixed value or calculate a new dueTime value based on the actual amount of time that expired, either are reasonable choices.

answered Apr 22 '14 at 21:22

Hans Passant

922,412
146
1,693
2,536

Thanks, this sounds like a nice simple option. I'm just trying to work it out though, isn't the CallBack asynchronous? So wouldn't that get run and then straight after the call it would call the timer.Change() - if I put it into the time_Elapsed method)? Which would then allow the timer to fire again without the CallBack having completed? Or am I wrong in saying the CallBack (Action) is asynchronous? – Richard Apr 22 '14 at 21:54
Well, sure, the callback is asynchronous. That's the point of using a Timer. You have the put the Change() call in the callback method. At the very end of the method, when you're done executing the code that takes time. – Hans Passant Apr 22 '14 at 22:02

Gusman · Answer 3 · 2014-04-23T03:41:43.813

0

you can use a boolean flag to prevent reentrance:

    bool executing;

    public TestOperation(Action callBackMethod)
    {
        this.timer = new System.Threading.Timer(timer_Elapsed, callbackMethod, timerInterval, Timeout.Infinite);
    }

    private void timer_Elapsed(object state)
    {
        if(executing)
             return;


        Action callback = (Action) state;
        if (callback != null)
        {
            executing = true;
            callback();
        }

    }

    // example of the callback, in another class. 
    private void callBackMethod()
    {
        // How can I stop this from running every 1 second? Lock() doesn't seem to work here

        Thread.Sleep(2000);
        executing = false;

    }

edited Apr 23 '14 at 03:41

answered Apr 22 '14 at 21:09

Gusman

14,905
2
34
50

1

If you have the lock, the Boolean flag is unnecessary. The `executing` flag is only modified when the lock is held. In addition, `lock(this)` is pretty terrible practice. I strongly recommend that anybody who sees this answer *not* do what is suggested. This is not a good solution at all. – Jim Mischel Apr 23 '14 at 01:40
1

Wire cross, yes, the lock must be removed (if lock is used and the timer is a threading timer, events will raise even if it's locked, and lots of aclls will be enqueued), also, a question @JimMischel, why do you say using lock(this) is a bad practice? if you are only doing it in controlled parts it ensures that you never will call lock() to a null object, any real concern about it? – Gusman Apr 23 '14 at 03:46

Calling Method only once from a Threading.Timer

3 Answers3