Windows service with AutoResetEvent

Question

I am currently creating a Windows service that should process a queue of messages that are in a database table. This queue can vary in length and can take anywhere from 5 seconds to 55 seconds to run against all rows in the database (I am currently using a test data set of 500,000 records)

The Windows service is configured to start with a 30 second timer, so I tried unsuccessfully to make sure that when the timer delegate starts, that it cannot work again until the previous method request completes successfully

I have the following code in my Windows OnStart method:

AutoResetEvent autoEvent = new AutoResetEvent(false); TimerCallback timerDelegate = new TimerCallback(MessageQueue.ProcessQueue); Timer stateTimer = new Timer(timerDelegate, autoEvent, 1000, Settings.Default.TimerInterval); // TimerInterval is 30000 autoEvent.WaitOne(); 

And the following code in MessageQueue.ProcessMessage:

  Trace.Write("Starting ProcessQueue"); SmtpClient smtp = new SmtpClient("winprev-01"); AutoResetEvent autoEvent = (AutoResetEvent)stateObject; foreach (MessageQueue message in AllUnprocessed) { switch (message.MessageType) { case MessageType.PlainText: case MessageType.HTML: SendEmail(smtp, message); break; case MessageType.SMS: SendSms(message); break; default: break; } } autoEvent.Set(); Trace.Write("Ending ProcessQueue"); 

I use DebugView to analyze the presentation of Trace instructions as the Service starts up, and I see several instances of the “Starting ProcessQueue” that occur every 30 seconds, which I am trying to avoid.

In short: I want to call ProcessQueue and make sure that it does not run again if it has not completed its work (this allows me to prevent several times the same messages from being processed in the queue

I'm sure I am missing something pretty obvious here, so any help would be greatly appreciated :)

Dave

Answer 1

Why don't you turn off your timer delegate and then turn it back on (or continue working if the timer expires immediately) after it works. If the delay between starting the timer and waking up the delegate is <30 seconds, this should be waterproof.

 while (true) { Trace.Write("Starting ProcessQueue") stateTimer.Enabled = false; DateTime start = DateTime.Now; // do the work // check if timer should be restarted, and for how long TimeSpan workTime = DateTime.Now - start; double seconds = workTime.TotalSeconds; if (seconds > 30) { // do the work again continue; } else { // Restart timer to pop at the appropriate time from now stateTimer.Interval = 30 - seconds; stateTimer.Enabled = true; break; } } 

Answer 2

Your ProcessMessage never checks if there is a resetEvent signal - it just works independently.

I am writing here how to fix it. However, this is not an ideal way to do what you want to do. See the bottom of my answer.

You are calling autoEvent.WaitOne() in the wrong place; it should be at the beginning of the ProcessMessage method.

 AutoResetEvent autoEvent = (AutoResetEvent)stateObject; autoEvent.WaitOne(); Trace.Write("Starting ProcessQueue"); SmtpClient smtp = new SmtpClient("winprev-01"); foreach (MessageQueue message in AllUnprocessed){ 

You should also use an overload that takes a timeout value (int or timespan) and returns a bool If the method returns true , it means that it has been signaled, so you can continue. If time runs out (because another iteration is still running), you should just go back and not try to run the code again.

If you do not use such overloading, then what you are doing is no different from transferring the code of the ProcessMessage method in the critical section ( lock() on the global var, for example) - additional threads block and then it is useless to work.

 AutoResetEvent autoEvent = (AutoResetEvent)stateObject; //wait just one ms to see if it gets signaled; returns false if not if(autoEvent.WaitOne(1)){ Trace.Write("Starting ProcessQueue"); SmtpClient smtp = new SmtpClient("winprev-01"); foreach (MessageQueue message in AllUnprocessed){ 

Note that actually a *ResetEvent is not ideal here. You really want to check if the instance is running, and abort it, if so. resetEvent not really created for this ... but I still wanted to resolve the issue of using ResetEvent.

What would be better to work is to simply turn off the timer when calling callback, and then restart it when you are done. Thus, it is not possible for this code to be re-entered while it is running.

You absolutely will need to wrap all the code in the callback method in try / finally , though, so that you always restart the timer after.

Answer 3

You can trivially solve this problem with System.Threading.Timer. You make it a one-shot timer, setting its period to zero. Restart the timer in the callback. Complex callbacks are no longer possible.

Since you run this so often, another approach is to use a thread instead. You will need AutoResetEvent to signal that the stream has stopped in the OnStop () method. Its WaitOne () method gives you a free timer when using overload, which takes a millisecondsTimeout argument.

Btw: note that calling autoEvent.WaitOne () on OnStart () is problematic. This may go to the service controller if the first email takes a long time to send. Just omit it, you started the timer == service timer.

Answer 4

OnStart Method

 AutoResetEvent autoEvent = new AutoResetEvent(true); while (true) { autoEvent.WaitOne(); Thread t = new Thread(MessageQueue.ProcessMessage); t.Start(autoEvent); } 

Answer 5

What you want is a synchronization timer object. In Win32, this is known as the expected timer (unfortunately, some P / invoke is required if I am not mistaken).

Here is what you would do:

• Create the expected timer (make sure it is automatically reset).
• Set the expected timer with a period of 30 seconds.
• Loop:
• WaitForSingleObject (expected timer) with an infinite timeout.
• process queue.

If processing takes more than 30 seconds, the timer will simply remain set until WaitForSingleObject is called on it. In addition, if processing takes 20 seconds, for example, a timer will be signaled after 10 seconds.

Answer 6

I think you make it a lot harder than necessary. Why not just create a separate thread that revolves around an infinite loop calling MessageQueue.ProcessQueue , and then waiting for a certain amount of time before calling it again. If all this happens in one thread, there is no way for something to happen in parallel.

 public class YourService : ServiceBase { private ManualResetEvent m_Stop = new ManualResetEvent(false); protected override void OnStart(string[] args) { new Thread(Run).Start(); } protected override void OnStop() { m_Stop.Set(); } private void Run() { while (!m_Stop.WaitOne(TimeSpan.FromSeconds(30)) { MessageQueue.ProcessMessage(); } } }