System.Timers.Timer is not stopping-CodePudding

I have created a System.Timers.Timer and im quite new to it. So I have used the Windows Forms Timer before and now I want to use this timer for a console application.
My problem is that my timer is not stopping after one OnTimeEvent() and I don't know why.

class SortingSysFinal
{
    private static System.Timers.Timer aTimer;

    public static void Main(String[] args)
    {
        System.Timers.Timer aTimer = new System.Timers.Timer();
        aTimer.Elapsed  = OnTimedEvent;
        aTimer.Interval = 1000;
        aTimer.Enabled = true;
            
        Console.Read();
    }   
        
    private static void OnTimedEvent(object source, ElapsedEventArgs e)
    {
        Console.WriteLine("The Elapsed event was raised at {0:HH:mm:ss.fff}", e.SignalTime);
        aTimer.Stop();
    }
}

CodePudding user response：

The problem is that in this line in your Main method:

System.Timers.Timer aTimer = new System.Timers.Timer();

You are creating a new local Timer object which is not the aTimer static member of your class. OnTimedEvent does get called, but it in this line: aTimer.Stop() it is attempting to use the aTimer static member which is null due to missing initialization (and therefore throws an exception).

To fix it, change the line above to:

aTimer = new System.Timers.Timer();

This will initialize the static member of your class as I believe you intended, and will enable the handler to stop the timer.

Note that System.Timers.Timer is not thread-safe. The Elapsed handler is called on a separate thread (see: documentation), and therefore you need to synchronize calline Stop() with a lock, or even better: use System.Threading.Timer instead.

CodePudding user response：

This line throws a NullReferenceException:

aTimer.Stop();

The reason is that the static field aTimer is not initialized. You might think that you initialized it in the Main method, by you didn't. You assigned the new System.Timers.Timer() to a local variable, instead of the static field. Here are some suggestions and remarks:

Name your fields with an underscore _ as prefix. For example _timer. This is a widely adopted practice, and helps immensely at differentiating between fields and local variables.
In this case you don't need a static field. You can get a reference to the timer by casting the object source argument: var timer = (System.Timers.Timer)source;.
Be aware that any exception thrown by the Elapsed event handler is suppressed. Essentially there is a giant and invisible try { /*...*/ } catch { } around your code, that prevents the exception form surfacing. According to the docs, "this behavior is subject to change in future releases of .NET Framework". The chances of this behavior ever changing are zero. Despite what Microsoft says, they never do breaking changes like this.
The System.Timers.Timer is not thread-safe. Also it is not impossible for the Elapsed event to be invoked in overlapping manner. So calling the Stop method inside the event handler, results in undefined behavior (by the docs). If you want to play by the book, you should synchronize the Stop call (and all other interactions with the timer) with a lock.

My suggestion is to not use the System.Timers.Timer class. IMHO it's not a well designed component. Use a System.Threading.Timer instead. Just make sure that your System.Threading.Timers are rooted (you store a reference to them somewhere), because otherwise they are eligible for garbage collection (unlike the System.Timers.Timer which is rooted by itself).

CodePudding user response：

You can set the AutoReset property to false (the default is true).

This will ensure your callback only gets invoked once.