Is the System.Timers.Timer.Enabled property safe and therefore could it be accessed from the timer Elapsed event?

I have an Elapsed method in which I have a while loop. If the timer is disabled / stopped from another thread, I would like this loop to stop. Can I rely on the Timer Enabled property in the Elapsed method for this or do I just need to create a "volatile bool timerEnabled" variable. My testimony shows that everything is in order, but I would like to be sure of this before putting it into production.

This is what I am trying to achieve (not the actual code, but close)

private volatile bool isElapsedAlreadyRunning

void myTimer_Elapsed(object sender, ElapsedEventArgs e)
{
    if (!isElapsedAlreadyRunning)   // to prevent reentrance
    {
        isElapsedAlreadyRunning = true;
        try 
        {
            while (myTimer.Enabled && some other condition)
            {
                do stuff
            }
        }
        finally
        {
            isElapsedAlreadyRunning = false;
        }
    }
}

myTimer.Start() and myTimer.Stop() are in other methods that can be called frrom other threads

      

I am using System.Timers.Timer class

If you have any comments or any mistakes in this project, feel free to comment :)

thank


Edit:

Man, the carving is heavy. Based on the answers and other stackoverflow questions ( this answer especially ) this would be the way to do it (hopefully it's okay this time)

public class NoLockTimer : IDisposable
{
    private readonly System.Timers.Timer _timer;

    private bool _isTimerStopped = false;
    private readonly object _isTimerStoppedLock = new object();

    public NoLockTimer()
    {
        _timer = new System.Timers.Timer { AutoReset = false, Interval = 1000 };

        _timer.Elapsed += delegate
        {
            try
            {
                while (!IsTimerStopped && some other condition) 
                {
                    // do stuff
                }
            }
            catch (Exception e)
            {
                // Do some logging
            }
            finally
            {
                if (!IsTimerStopped) 
                {
                    _timer.Start(); // <- Manual restart.
                }
            }
        };

        _timer.Start();
    }

    public void Stop()
    {
        IsTimerStopped = true;
        if (_timer != null)
        {
            _timer.Stop();
        }
    }

    private bool IsTimerStopped
    {
        get 
        {
            lock (_isTimerStoppedLock)
            {
                return _isTimerStopped;
            }
        }
        set 
        {
            lock (_isTimerStoppedLock)
            {
                _isTimerStopped = value;
            }
        }
    }

    public void Dispose()
    {
        Stop();
        if (_timer != null)
        {
            _timer.Dispose();
        }
    }
} 

      

+3


source to share


2 answers


No, it is not safe. The Elapsed event handler is called on the threadpool. You cannot predict when this thread will actually call your method, it depends on what other TP threads are running in the process. It is technically possible to have two calls in flight at the same time. What makes the volatile keyword of isElapsedAlreadyRunning not good enough to guarantee that the method is thread safe, you should use the lock keyword or Monitor.TryEnter () instead.

This issue goes away if the Timer AutoReset property is set to false. Make sure to restart the timer in the finally block, another annoying problem with the Timer.Elapsed event is that exceptions are swallowed without diagnostics. System.Threading.Timer is a versatile best timer with fewer surprises.



The Timer.Enabled property has a similar problem, you will always see it late.

+6


source


Your defender c isElapsedAlreadyRunning

is obviously not thread safe.



But you can simply replace it with an instruction lock(...) { ...}

.

+1


source







All Articles