With reference to this quote from MSDN about the System.Timers.Timer:
The Timer.Elapsed event is raised on a
ThreadPool thread, so the
event-handling method might run on one
thread at the same time that a call to
the Timer.Stop method runs on another
thread. This might result in the
Elapsed event being raised after the
Stop method is called. This race
condition cannot be prevented simply
by comparing the SignalTime property
with the time when the Stop method is
called, because the event-handling
method might already be executing when
the Stop method is called, or might
begin executing between the moment
when the Stop method is called and the
moment when the stop time is saved. If
it is critical to prevent the thread
that calls the Stop method from
proceeding while the event-handling
method is still executing, use a more
robust synchronization mechanism such
as the Monitor class or the
CompareExchange method. Code that uses
the CompareExchange method can be
found in the example for the
Timer.Stop method.
Can anyone give an example of a "robust synchronization mechanism such as the Monitor class" to explain what this means exactly?
I am thinking it means use a lock somehow, but I am unsure how you would implement that.
Stopping a System.Timers.Timer reliably is indeed a major effort. The most serious problem is that the threadpool threads that it uses to call the Elapsed event can back up due to the threadpool scheduler algorithm. Having a couple of backed-up calls isn't unusual, having hundreds is technically possible.
You'll need two synchronizations, one to ensure you stop the timer only when no Elapsed event handler is running, another to ensure that these backed-up TP threads don't do any harm. Like this:
System.Timers.Timer timer = new System.Timers.Timer();
object locker = new object();
ManualResetEvent timerDead = new ManualResetEvent(false);
private void Timer_Elapsed(object sender, ElapsedEventArgs e) {
lock (locker) {
if (timerDead.WaitOne(0)) return;
// etc...
}
}
private void StopTimer() {
lock (locker) {
timerDead.Set();
timer.Stop();
}
}
Consider setting the AutoReset property to false. That's brittle another way, the Elapsed event gets called from an internal .NET method that catches Exception. Very nasty, your timer code stops running without any diagnostic at all. I don't know the history, but there must have been another team at MSFT that huffed and puffed at this mess and wrote System.Threading.Timer. Highly recommended.
That is what it is suggesting.
Monitor
is the class that's used by the C# compiler for a lock
statement.
That being said, the above is only a problem if it is an issue in your situation. The entire statement basically translates to "You could get a timer event that happens right after you call Stop(). If this is a problem, you'll need to deal with it." Depending on what your timer is doing, it may be an issue, or it may not.
If it's a problem, the Timer.Stop page shows a robust way (using Interlocked.CompareExchange) to handle this. Just copy the code from the sample and modify as necessary.
Try:
lock(timer) {
timer.Stop();
}
Here is a very simple way to prevent this race condition from occurring:
private object _lock = new object();
private Timer _timer; // init somewhere else
public void StopTheTimer()
{
lock (_lock)
{
_timer.Stop();
}
}
void elapsed(...)
{
lock (_lock)
{
if (_timer.Enabled) // prevent event after Stop() is called
{
// do whatever you do in the timer event
}
}
}
Seems timer is not thread safe. You must keep all calls to it in sync via locking. lock(object){} is actually just short hand for a simple monitor call.