CODEWITHSUNDEEP
c#.netmultithreading
krembanan
krembanan

Reputation: 1428

Monitor, lock or volatile?

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.

Upvotes: 5

Views: 468

Answers (3)

Nikita B
Nikita B

Reputation: 3333

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))
            {
                 ..........
            }
         });

Upvotes: 0

Boris B.
Boris B.

Reputation: 5024

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);
    }
}

Upvotes: 6

Marc Gravell
Marc Gravell

Reputation: 1062770

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);
}

Upvotes: 1

Related Questions