views:

394

answers:

2

I wrote a Timer class in order to use it within a windows service that is polling another system. I did this because I had two issues that the System.Timers.Timer is not addressing.

  1. The Elapsed EventHanler is running in background and so its execution will abort if the main thread ends. I wanted the System.Timers.Timer.Stop function to block the main thread until the execution of the Elapsed EventHanler had end.
  2. System.Timers.Timer is not dealing with event reentrance. I want the Interval to be between two Elapsed EventHanler so that the Timer wont ever call the Elapsed EventHanler if the previous call (+interval) had not finished yet.

While writing the class I found out that I would need to dael with some thrading related problems and since I'm not too experienced at those I want to know if the following Timer class is Thread-Safe?

public class Timer
{
    System.Timers.Timer timer = new System.Timers.Timer() { AutoReset = false };
    ManualResetEvent busy = new ManualResetEvent(true);

    public double Interval
    {
        get { return timer.Interval; }
        set { timer.Interval = value; }
    }

    public Timer()
    {
        timer.Elapsed += new ElapsedEventHandler(TimerElapsed);
    }

    void TimerElapsed(object sender, ElapsedEventArgs e)
    {
        try
        {
            busy.Reset();
            OnElapsed(e);
            timer.Start();
        }
        finally
        {
            busy.Set();
        }
    }

    public event EventHandler Elapsed;

    protected void OnElapsed(EventArgs e)
    {
        if (Elapsed != null)
        {
            Elapsed(this, e);
        }
    }

    public virtual void Start()
    {
        busy.WaitOne();
        timer.Start();
    }

    public virtual void Stop()
    {
        busy.WaitOne();
        timer.Stop();
    }
}
+2  A: 

First of all you can use System.Threading.Timer instead of this timer as according to my experience that is a better performing timer (just an advise on personal experience).

Secondly in such cases you should give a flag which is set once the earlier timer has completed the task (this flag - a static field which is accessed by all threads).

In this case please ensure that in case of any error, then also the flag is reset by you so that the other timer does not wait infinitely in case timer task is not able to set the flag for other timers due to the error occurred inside the task (sort of final block should be added to ensure error is handled and flag is always reset).

Once this flag is reset then the next thread works on it so this check would ensure that all threads start task on the same one by one.

Sample Code which I wrote for situation like this (methods code has been removed, this will give the design details to you).

namespace SMSPicker
{
 public partial class SMSPicker : ServiceBase{
    SendSMS smsClass;
    AutoResetEvent autoEvent;
    TimerCallback timerCallBack;
    Timer timerThread;
    public SMSPicker()
    {
        InitializeComponent();
    }

    protected override void OnStart(string[] args)
    {
        // TODO: Add code here to start your service.
        smsClass = new SendSMS();
        autoEvent = new AutoResetEvent(false);
        long timePeriod = string.IsNullOrEmpty(ConfigurationSettings.AppSettings["timerDuration"]) ? 10000 : Convert.ToInt64(ConfigurationSettings.AppSettings["timerDuration"]);
        timerCallBack = new TimerCallback(sendSMS);
        timerThread = new Timer(timerCallBack, autoEvent, 0, timePeriod);
    }


    private void sendSMS(object stateInfo)
    {
        AutoResetEvent autoResetEvent = (AutoResetEvent)stateInfo;
        smsClass.startSendingMessage();
        autoResetEvent.Set();
     }

    protected override void OnStop()
    {
        // TODO: Add code here to perform any tear-down necessary to stop your service.
        smsClass.stopSendingMessage();
        timerThread.Dispose();            

    }
}
}







namespace SMSPicker
{
class SendSMS
{
 //This variable has been done in order to ensure that other thread does not work till this thread ends
    bool taskDone = true;
 public SendSMS()
    {

    }

    //this method will start sending the messages by hitting the database
    public void startSendingMessage()
    {

        if (!taskDone)
        {
            writeToLog("A Thread was already working on the same Priority.");
            return;
        }

        try
        {
        }
        catch (Exception ex)
        {
            writeToLog(ex.Message);
        }
        finally
        {
            taskDone = stopSendingMessage();

            //this will ensure that till the database update is not fine till then, it will not leave trying to update the DB
            while (!taskDone)//infinite looop will fire to ensure that the database is updated in every case
            {
                taskDone = stopSendingMessage();
            }
        }

    }


public bool stopSendingMessage()
    {
        bool smsFlagUpdated = true;
        try
        {

        }
        catch (Exception ex)
        {
            writeToLog(ex.Message);
        }
        return smsFlagUpdated;
    }

}
}
Beginner
A: 

I had a similar issue: the simpler thing to do could be to use System.Threading.Timer with period=0;

On this scenario, the call back method is invoked once. Then, resetting the timer (calling its Change() method) turns it on again; at the end of your re-occurring method.

This scenario is explained here: http://kristofverbiest.blogspot.com/2008/08/timers-executing-task-at-regular.html

Francisco
this solution does not solve the issue described in 1.
Martin