Monitor, lock or volatile?

Question

I have a windows service (.NET 4) that periodically processes a queue, for example every 15 minutes. I use a System.Threading.Timer which is set when the service starts to fire a callback every X milliseconds. Typically each run takes seconds and never collides, but what if I could not assume that - then I want the next run to exit at once if processing is in progress.

This is easily solved with lock, volatile bool or a monitor, but what is actually the appropriate to use in this scenario, or simply the preferred option in general?

I've found other posts that answers almost this scenario (like Volatile vs. Interlocked vs. lock) but need some advice on extending this to a Timer example with immediate exit.

Boris B. · Accepted Answer · 2013-06-24T09:58:13.240

6

You don't need any locks for this, you should just reschedule next timer execution from within the timer delegate. That should ensure 100% no overlaps.

At the end of timer's event handler call timer.Change(nextRunInMilliseconds, Timeout.Infinite), that way the timer will fire only once, after nextRunInMilliseconds.

Example:

//Object that holds timer state, and possible additional data
private class TimerState
{
    public Timer Timer { get; set; }
    public bool Stop { get; set; }
}

public void Run()
{
    var timerState = new TimerState();
    //Create the timer but don't start it
    timerState.Timer = new Timer(OnTimer, timerState, Timeout.Infinite, Timeout.Infinite);
    //Start the timer
    timerState.Timer.Change(1000, Timeout.Infinite);
}

public void OnTimer(object state)
{
    var timerState = (TimerState) state;            
    try
    {
        //Do work
    }
    finally 
    {
        //Reschedule timer
        if (!timerState.Stop)
            timerState.Timer.Change(1000, Timeout.Infinite);
    }
}

edited Jun 24 '13 at 09:58

answered Jun 24 '13 at 09:10

Boris B.

4,933
1
28
59

1

+1, better to prevent overlap in the first place. But shouldn't this be combined with *disabling* the timer completely at the *start* of processing, using `timer.Change(Timeout.Infinite, Timeout.Infinite)`? – anton.burger Jun 24 '13 at 09:31
@shambulator: +1 for the disabling of timer at the start of the eventHandler. So typically, something like "timer.Change(Timeout.Infinite, Timeout.Infinite)" at start and during the end of the event handler (After the processing cycle) restore the timer to its initial state.. – now he who must not be named. Jun 24 '13 at 09:39
2

@shambulator: The timer *is* disabled at the start of the processing. Passing `Timeout.Infinite` as the `period` param will cause the timer to fire only once, so once the delegate fires the timer is already *disabled*. – Boris B. Jun 24 '13 at 09:44
@BorisB. Ah, that makes sense :) My mistake was in assuming that the timer is initially set to recur, but your edit makes clear that it starts life as a one-shot timer and is renewed after every interval has elapsed. – anton.burger Jun 24 '13 at 10:25

score 1 · Answer 2 · answered Jun 24 '13 at 09:11

Well, any of them will do the job. Monitor is usually pretty simple to use via lock, but you can't use lock in this case because you need to specify a zero timeout; as such, the simplest approach is probably a CompareExchange:

private int isRunning;
...
if(Interlocked.CompareExchange(ref isRunning, 1, 0) == 0) {
    try {
        // your work
    } finally {
        Interlocked.Exchange(ref isRunning, 0);
    }
}

to do the same with Monitor is:

private readonly object syncLock = new object();
...
bool lockTaken = false;
try {
    Monitor.TryEnter(syncLock, 0, ref lockTaken);
    if (lockTaken) {
        // your work
    }
} finally {
    if(lockTaken) Monitor.Exit(syncLock);
}

Nikita B · Answer 3 · 2013-06-24T10:05:25.107

I think, that if you find that you need to synchronize timer delegate - you are doing it wrong, and Timer is probably not the class you want to use. Imho its better to :

1) either keep the Timer, but increase the interval value to the point, where its safe to assume, that there will be no issues with threading,

2) or remove Timer and use simple Thread instead. You know, something like:

var t = new Thread();
t.Start(() =>
         {
            while (!_stopEvent.WaitOne(100))
            {
                 ..........
            }
         });

Monitor, lock or volatile?

3 Answers3