Reliably stop System.Threading.Timer?

Question

Well I've searched a lot for a solution to this. I'm looking for a clean and simple way to prevent the callback method of a System.Threading.Timer from being invoked after I've stopped it.

I can't seem to find any, and this has led me, on occassion, to resort to the dreaded thread-thread.sleep-thread.abort combo shudders.

Can it be done using lock? Please help me find a good way to do this. Thanks

Answer 1

For what it's worth, we use this pattern quite a bit:

// set up timer
Timer timer = new Timer(...);
...

// stop timer
timer.Dispose();
timer = null;
...

// timer callback
{
  if (timer != null)
  {
    ..
  }
}

Answer 2

To me, this seems to be the correct way to go: Just call dispose when you are done with the timer. That will stop the timer and prevent future scheduled calls.

See example below.

class Program
{
    static void Main(string[] args)
    {
        WriteOneEverySecond w = new WriteOneEverySecond();
        w.ScheduleInBackground();
        Console.ReadKey();
        w.StopTimer();
        Console.ReadKey();
    }
}

class WriteOneEverySecond
{
    private Timer myTimer;

    public void StopTimer()
    {
        myTimer.Dispose();
        myTimer = null;
    }

    public void ScheduleInBackground()
    {
        myTimer = new Timer(RunJob, null, 1000, 1000);
    }

    public void RunJob(object state)
    {
        Console.WriteLine("Timer Fired at: " + DateTime.Now);
    }
}

Answer 3

This answer relates to System.Threading.Timer

I've read a lot of nonsense about how to synchronize disposal of System.Threading.Timer all over the net. So that's why I'm posting this in an attempt to rectify the situation somewhat. Feel free to tell me off / call me out if something I'm writing is wrong ;-)

Pitfalls

In my opinion there's these pitfalls:

• Timer.Dispose(WaitHandle) can return false. It does so in case it's already been disposed (i had to look at the source code). In that case it won't set the WaitHandle - so don't wait on it!
• not handling a WaitHandle timeout. Seriously - what are you waiting for in case you're not interested in a timeout?
• Concurrency issue as mentioned here on msdn where an ObjectDisposedException can occur during (not after) disposal.
• Timer.Dispose(WaitHandle) does not work properly with -Slim waithandles, or not as one would expect. For example, the following does not work (it blocks forever):

 using(var manualResetEventSlim = new ManualResetEventSlim)
 {
     timer.Dispose(manualResetEventSlim.WaitHandle);
     manualResetEventSlim.Wait();
 }

Solution

Well the title is a bit "bold" i guess, but below is my attempt to deal with the issue - a wrapper which handles double-disposal, timeouts, and ObjectDisposedException. It does not provide all of the methods on Timer though - but feel free to add them.

internal class Timer
{
    private readonly TimeSpan _disposalTimeout;

    private readonly System.Threading.Timer _timer;

    private bool _disposeEnded;

    public Timer(TimeSpan disposalTimeout)
    {
        _disposalTimeout = disposalTimeout;
        _timer = new System.Threading.Timer(HandleTimerElapsed);
    }

    public event Signal Elapsed;

    public void TriggerOnceIn(TimeSpan time)
    {
        try
        {
            _timer.Change(time, Timeout.InfiniteTimeSpan);
        }
        catch (ObjectDisposedException)
        {
            // race condition with Dispose can cause trigger to be called when underlying
            // timer is being disposed - and a change will fail in this case.
            // see 
            // https://msdn.microsoft.com/en-us/library/b97tkt95(v=vs.110).aspx#Anchor_2
            if (_disposeEnded)
            {
                // we still want to throw the exception in case someone really tries
                // to change the timer after disposal has finished
                // of course there's a slight race condition here where we might not
                // throw even though disposal is already done.
                // since the offending code would most likely already be "failing"
                // unreliably i personally can live with increasing the
                // "unreliable failure" time-window slightly
                throw;
            }
        }
    }

    private void HandleTimerElapsed(object state)
    {
        Elapsed.SafeInvoke();
    }

    public void Dispose()
    {
        using (var waitHandle = new ManualResetEvent(false))
        {
            // returns false on second dispose
            if (_timer.Dispose(waitHandle))
            {
                if (!waitHandle.WaitOne(_disposalTimeout))
                {
                    throw new TimeoutException(
                        "Timeout waiting for timer to stop. (...)");
                }
                _disposeEnded = true;
            }
        }
    }
}

Answer 4

For the System.Threading.Timer one can do the following (Will also protect the callback-method from working on a disposed timer - ObjectDisposedException):

class TimerHelper : IDisposable
{
    private System.Threading.Timer _timer;
    private readonly object _threadLock = new object();

    public event Action<Timer,object> TimerEvent;

    public void Start(TimeSpan timerInterval, bool triggerAtStart = false,
        object state = null)
    {
        Stop();
        _timer = new System.Threading.Timer(Timer_Elapsed, state,
            System.Threading.Timeout.Infinite, System.Threading.Timeout.Infinite);

        if (triggerAtStart)
        {
            _timer.Change(TimeSpan.FromTicks(0), timerInterval);
        }
        else
        {
            _timer.Change(timerInterval, timerInterval);
        }
    }

    public void Stop(TimeSpan timeout = TimeSpan.FromMinutes(2))
    {
        // Wait for timer queue to be emptied, before we continue
        // (Timer threads should have left the callback method given)
        // - http://woowaabob.blogspot.dk/2010/05/properly-disposing-systemthreadingtimer.html
        // - http://blogs.msdn.com/b/danielvl/archive/2011/02/18/disposing-system-threading-timer.aspx
        lock (_threadLock)
        {
            if (_timer != null)
            {
                ManualResetEvent waitHandle = new ManualResetEvent(false)
                if (_timer.Dispose(waitHandle))
                {
                   // Timer has not been disposed by someone else
                   if (!waitHandle.WaitOne(timeout))
                      throw new TimeoutException("Timeout waiting for timer to stop");
                }
                waitHandle.Close();   // Only close if Dispose has completed succesful
                _timer = null;
            }
        }
    }

    public void Dispose()
    {
        Stop();
        TimerEvent = null;
    }

    void Timer_Elapsed(object state)
    {
        // Ensure that we don't have multiple timers active at the same time
        // - Also prevents ObjectDisposedException when using Timer-object
        //   inside this method
        // - Maybe consider to use _timer.Change(interval, Timeout.Infinite)
        //   (AutoReset = false)
        if (Monitor.TryEnter(_threadLock))
        {
            try
            {
                if (_timer==null)
                    return;

                Action<Timer, object> timerEvent = TimerEvent;
                if (timerEvent != null)
                {
                    timerEvent(_timer, state);
                }
            }
            finally
            {
                Monitor.Exit(_threadLock);
            }
        }
    }
}

This is how one can use it:

void StartTimer()
{
    TimerHelper _timerHelper = new TimerHelper();
    _timerHelper.TimerEvent += (timer,state) => Timer_Elapsed();
    _timerHelper.Start(TimeSpan.FromSeconds(5));
    System.Threading.Sleep(TimeSpan.FromSeconds(12));
    _timerHelper.Stop();
}

void Timer_Elapsed()
{
   // Do what you want to do
}

Answer 5

like Conrad Frix suggested you should use the System.Timers.Timer class instead, like:

private System.Timers.Timer _timer = new System.Timers.Timer();
private volatile bool _requestStop = false;

public constructor()
{
    _timer.Interval = 100;
    _timer.Elapsed += OnTimerElapsed;
    _timer.AutoReset = false;
    _timer.Start();
}

private void OnTimerElapsed(object sender, System.Timers.ElapsedEventArgs e)
{
    // do work....
    if (!_requestStop)
    {
        _timer.Start();//restart the timer
    }
}

private void Stop()
{
    _requestStop = true;
    _timer.Stop();
}

private void Start()
{
    _requestStop = false;
    _timer.Start();
}