Reentrant Timer in Windows Service

Question

I want to build a windows Service, which should execute different methods at different times. Its not about accuracy at all. Im using a system.timers.timer, and regulate the different methods to be executed within the Eventhandler-method with counters. Thats working allright that far.

All of the methods are accessing a COM-port, making it neccessary to grant acceess-rights to only one method at a time. But since the methods can take some time to finish, the timer might tick again and want to execute another method while the COM-port is still being occupied. In this case, the event can and should just be dismissed.

Simplified down to one method, my elapsedEventHandler-method looks something like the following (try-catch and the different methods excluded here)

Note: While this is running perfectly on my Win7 x64, it struggles on a Win7 x86 machine with pretty much the very same software installed, whenever the method to be executed takes a long time. The timer wont tick any more, no Exception is thrown. Nothing! my question now is: Am I doing the part with access-control and the timer right, so that i can focus on other things? Im just not that familiar with timers and especially threading

     private static int m_synchPoint=0;
     private System.Timers.Timer timerForData = null;

    public MyNewService()
    {

        timerForData = new System.Timers.Timer();
        timerForData.Interval = 3000;
        timerForData.Elapsed += new ElapsedEventHandler(Timer_tick);
    }
    //Initialize all the timers, and start them
    protected override void OnStart(string[] args)
    {

        timerForData.AutoReset = true;
        timerForData.Enabled = true;
        timerForData.Start();
    }

    //Event-handled method
    private void Timer_tick(object sender, System.Timers.ElapsedEventArgs e)
    {
            ////safe to perform event - no other thread is running the event?                      
            if (System.Threading.Interlocked.CompareExchange(ref m_synchPoint, 1, 0) == 0)

            {
             //via different else-ifs basically always this is happening here, except switching aMethod,bMethod...
             processedevent++; 
             Thread workerThread = new Thread(aMethod);
             workerThread.Start();
             workerThread.Join(); 
             m_synchPoint=0;
             }
             else
             {
              //Just dismiss the event
              skippedevent++;
             }
     }

Thank you very much in advance!
Any help is greatly appreciated!

Actually i CANT repro the problem on my machine, only on the machines i am testing on, where i cant debug in Visual Studio. The main question here is the following: Does the above code make correct use of timers and Compare Exchange, that its save to assume, that only ONE Method at a time is operating? Else: How would you implement such a scenario? — Timo Jak, Dec 10 '11 at 20:41
This isn't the core problem, but do *not* use `static` for data which should be instance-specific (as `m_synchPoint` should be in this case). — bobbymcr, Dec 10 '11 at 20:45
Why are you calling `workerThread.Start` followed immediately by `workerThread.Join`? There is no benefit to doing things this way, since the timer thread is just going to wait until the new thread is done. Execute `aMethod` directly, and save yourself the overhead of spinning up a new thread. — Jim Mischel, Dec 10 '11 at 21:08
JimMischel:Reading my log.debug, it appeared to me, that this wasnt the case, which surprised me as well! Is this possible at all? My initial thought was, to explicitly advise the service to wait for completion... @bobbymcr: Right of course but not the fix =( — Timo Jak, Dec 10 '11 at 22:12
I think you misunderstood. Rather than create a thread to execute `aMethod`, start the thread and wait for it to complete, eliminate the thread altogether and just call `aMethod` directly. — Jim Mischel, Dec 10 '11 at 22:16
I wasnt clear I think... I DO understand, what you are saying. What I meant was, that, when I did it the way you suggest, I saw from my debug-file that Timer_tick wouldnt wait for aMethod to complete, before it would finish itself. Is that possible at all? That made me include the new thread in the first place...to be able to explicitly say "wait" — Timo Jak, Dec 10 '11 at 22:37
If you call `aMethod` directly from the timer tick, the timer tick will not complete before `aMethod` exits. It's just code, after all. If you call a method on a thread--even in a timer tick, that method must complete before the caller can continue. — Jim Mischel, Dec 12 '11 at 04:41

Bryan Crosby · Answer 1 · 2011-12-11T17:19:34.290

4

I would recommend using System.Threading.Timer for this functionality. You can disable the timer when it executes, process your data, then re-enable the timer.

EDIT:

I think it makes more sense to use System.Threading.Timer because there isn't really a reason you need to drop the timer on a design surface, which is pretty much the only reason to use System.Timers.Timer. I really wish MS would remove it anyways, it's wrapping System.Threading.Timer which isn't all that difficult to use in the first place.

Yes, you do risk a problem with re-entrancy which is why I specified to change the timeout toTimeout.Infinite. You won't have this re-entrancy problem if you construct the timer with Timeout.Infinite.

public class MyClass
{
    private System.Threading.Timer _MyTimer;

public MyClass()
{
    _MyTimer = new Timer(OnElapsed, null, 0, Timeout.Infinite);
}

public void OnElapsed(object state)
{
    _MyTimer.Change(Timeout.Infinite, Timeout.Infinite);
    Console.WriteLine("I'm working");
    _MyTimer.Change(1000, Timeout.Infinite);
}

}

edited Dec 11 '11 at 17:19

answered Dec 10 '11 at 20:13

Bryan Crosby

6,486
3
36
55

But you can disable `System.Timers.Timer`, too. `Timer.Enabled = false`. – Jim Mischel Dec 10 '11 at 21:02
plus, I read that timerevents can be evoked, even after the timer has been stopped? Thats basically the reason i use CompareExchange [link](http://msdn.microsoft.com/en-us/library/system.timers.timer.stop.aspx) – Timo Jak Dec 10 '11 at 22:26
@JimMischel: Clarified my answer a bit. – Bryan Crosby Dec 11 '11 at 17:23
1

@Bryan: I, too, discourage the use of `System.Timers.Timer`, because it swallows exceptions, which hides bugs. That said, the wrapping is useful for two reasons: 1) it uses an event model rather than a callback. The event model is more familiar to .NET programmers in general. Second, if you're using it in a Windows Forms application, setting the `SynchronizingObject` to the form causes the event handler to be called on the GUI thread, relieving you of having to use `InvokeRequired` and `Invoke` in order to access controls. – Jim Mischel Dec 12 '11 at 04:37
@Timo: Yes, it's possible that a timer callback (or event) can be invoked after the timer has been stopped. But that can only happen if the next timer tick occurs before the previous tick's callback stops the timer. You would need a very fast timer or a very slow response for this to happen. – Jim Mischel Dec 12 '11 at 04:44
@Jim: Points taken. I only emphasized it in this scenario since he said he was building a Windows Service. – Bryan Crosby Dec 12 '11 at 16:37
@BryanCrosby: Thanks for the edit. Looks like the only valid reason for me using timers.timer is that i didnt know of ways to implement threading.timer. Because like I said, its a WindowsService, no GUI. I got this working with the monitor-method above, but anyways, Im going to change the timer, to fiddle around and see what i get. Thank you! – Timo Jak Dec 14 '11 at 09:54

score 2 · Answer 2 · answered Dec 10 '11 at 19:18

2

You can try this:

When the timer fires, disable the timer.

When the task is complete, re-enable the timer...possibly in the Finally clause.

answered Dec 10 '11 at 19:18

Steve Wellens

20,506
2
28
69

Maxim · Accepted Answer · 2011-12-12T22:32:20.787

2

If you want just skip method invocation while previous method didn't finish just use Monitor.TryEnter(lockObject) before calling your method.

EDIT: Here's an example -

public class OneCallAtATimeClass
{

    private object syncObject;

    public TimerExample()
    {
      syncObject = new object();
    }

    public void CalledFromTimer()
    {    
      if (Monitor.TryEnter(syncObject);)
      {
        try
        {
          InternalImplementation();
        }
        finally
        {
          Monitor.Exit(syncObject);
        }
      }    
    }

    private void InternalImplementation()
    {
      //Do some logic here
    }

  }

edited Dec 12 '11 at 22:32

answered Dec 10 '11 at 20:42

Maxim

7,268
1
32
44

Exactly my thought. You might add an example that shows calling `TryEnter` at the beginning of the method, and the correct `try...finally` that calls `Monitor.Exit` on completion. – Jim Mischel Dec 10 '11 at 21:06
@JimMischel - sure, added an example – Maxim Dec 11 '11 at 19:47

score 2 · Answer 4 · answered Dec 10 '11 at 20:49

2

You correctly use CompareExchange to test and set the m_synchPoint field when doing the initial check. You incorrectly use direct assignment to reset the value to 0 at the end of the method. You should use Interlocked.Exchange instead to reset the value to 0. As a side note, you should also change m_synchPoint to an instance field -- it should not be static.

answered Dec 10 '11 at 20:49

bobbymcr

23,769
3
56
67

Id say "useful" but i cant xD – Timo Jak Dec 10 '11 at 23:21

Reentrant Timer in Windows Service

4 Answers4