19

I have a Windows Service implemented in C# that needs to do some work every so often. I've implemented this using a System.Threading.Timer with a callback method that is responsible for scheduling the next callback. I am having trouble gracefully stopping (i.e. disposing) the timer. Here's some simplified code you can run in a console app that illustrates my problem:

const int tickInterval = 1000; // one second timer = new Timer( state => { // simulate some work that takes ten seconds Thread.Sleep( tickInterval * 10 ); // when the work is done, schedule the next callback in one second timer.Change( tickInterval, Timeout.Infinite ); }, null, tickInterval, // first callback in one second Timeout.Infinite ); // simulate the Windows Service happily running for a while before the user tells it to stop Thread.Sleep( tickInterval * 3 ); // try to gracefully dispose the timer while a callback is in progress var waitHandle = new ManualResetEvent( false ); timer.Dispose( waitHandle ); waitHandle.WaitOne();

The problem is that I get an ObjectDisposedException from timer.Change on the callback thread while waitHandle.WaitOne is blocking. What am I doing wrong?

The documentation for the Dispose overload I'm using says:

The timer is not disposed until all currently queued callbacks have completed.

Edit: It appears that this statement from the documentation may be incorrect. Can someone verify?

I know that I could work around the problem by adding some signaling between the callback and the disposal code as Henk Holterman suggested below, but I don't want to do this unless absolutely necessary.

Improve this question
9
  • Why can't you let the timer run by itself every 10 seconds? Why are you manually rescheduling it? Commented Sep 10, 2012 at 15:36
  • 1
    @Tudor: Because sometimes the work takes longer, and I don't want multiple callbacks overlapping in their execution. Commented Sep 10, 2012 at 15:37
  • Why are you manually exposing the timer when the service is stopped? When the AppDomain unloads all memory is re-claimed anyway. Commented Sep 10, 2012 at 15:38
  • @DanielHilgarth: In my real implementation, I'm doing the timer disposal from ServiceBase.OnStop. I want to make sure the timer is disposed and no callbacks are in progress before I let the service shut down. I also have other shut down code I need to run, and I don't want to do this until I'm sure the timer is completely gone. Commented Sep 10, 2012 at 15:43
  • I'm beginning to wonder if you are encountering a subtle problem tied to the Thread callback reference as the anonymous delegate to the constructor....what if you (for testing) converted it to a named function...given that the timer fires on a secondary CLR thread.. Commented Sep 10, 2012 at 16:28

4 Answers 4

Reset to default
12

With this code

 timer = new Timer( state => { // simulate some work that takes ten seconds Thread.Sleep( tickInterval * 10 ); // when the work is done, schedule the next callback in one second timer.Change( tickInterval, Timeout.Infinite ); }, null, tickInterval, // first callback in one second Timeout.Infinite );

it is almost certain that you will Dispose the timer while it is sleeping.

You will have to safeguard the code after Sleep() to detect a Disposed timer. Since there is no IsDisposed property a quick and dirty static bool stopping = false; might do the trick.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Thank you; I just thought I wouldn't need to do something like that since the documentation states "The timer is not disposed until all currently queued callbacks have completed." What does that sentence mean?
hat sentence means you should be right but I saw a ObjectDisposed exception on timer.Change(). Get rid of that one anyway, there are other ways to prevent re-entrance.
Wondering if perhaps the phrase "queued callbacks" literally means additional callbacks that may be pending, but not currently executing, eg a currently executing callback is, by definition, not queued?
@DavidW: Yeah, maybe. I am just going to work around this problem for now by catching and suppressing ObjectDisposedException from timer.Change in the callback. Maybe eventually someone from Microsoft will see this and give me a better answer.
You seem to have a reproducible case that contradicts the docs, consider posting it on MS Connect
0

As described in "Concurrent Programming on Windows":
Create a dummy class InvalidWaitHandle, inheriting from WaitHandle:

using System; using System.Collections.Generic; using System.Linq; using System.Text; using System.Threading.Tasks; using System.ComponentModel; using System.Data; using System.Diagnostics; using System.Threading; namespace MyNameSpace { class InvalidWaitHandle : WaitHandle { } }

Hence you can Dispose a System.Threading.Timer properly like this:

public static void DisposeTimer() { MyTimer.Dispose(new InvalidWaitHandle()); MyTimer = null; }
Improve this answer

1 Comment

What is the purpose of the dummy WaitHandle?
0

Possible solution for protecting the callback method from working on a disposed timer:

ManualResetEvent waitHandle = new ManualResetEvent(false); if (!timer.Dispose(waitHandle) || waitHandle.WaitOne((int)timeout.TotalMilliseconds) { waitHandle.Close(); // Only close when not timeout }

See also: https://stackoverflow.com/a/15902261/193178

Improve this answer

Comments

-5

You do not need to dispose of the timer to stop it. You can call Timer.Stop() or set Timer.Enabled to false, either of which will stop the timer from running.

Improve this answer

1 Comment

I believe those are members of System.Timers.Timer, not System.Threading.Timer, which I am using.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.