0

Assume we have this event attached to the timer event handler.

private void TimerTick(object sender, ElapsedEventArgs e)
{
    if(_gaurd) return;
    lock (this) // lock per instance
    {
        _gaurd = true;
        if (!_timer.Enabled) return;

        OnTick(); // somewhere inside here timer may pause it self.

        _gaurd = false;
    }
}

Now there two things that can pause this timer. One is user request from UI thread, second is the timer which may pause it self.

If the timer pause it self we can guarantee the pause will complete before we continue.

timer.Stop();
OnPause(); // timer must be paused because OnPause() is not thread safe.

But if the user, requests for timer pause the request is from another thread and we can not guarantee timer is fully paused or not.

timer.Stop();
OnPause(); // timer event may be still inside OnTick() and may conflict with OnPause()

-------------------------------------------------------------------------------------------------------

So I'm looking for a way to make this thread safe. This is what I have tried so far but I'm not sure if this works in all situations or not.

Its looking good but want to make sure that if there is anything I'm not aware of. or maybe to know if there are better ways to make this process thread safe.

I have tried to separate user request from Inner workings of timer. therefore I have two Pause methods for my timer.

public class Timer
{
    internal void InternalStop() // must be called by timer itself.
    {
        timer.Pause(); // causes no problem
    }

    public void Stop() // user request must come here. (if timer call this deadlock happens)
    {
        InternalStop();
        lock (this) // reference of timer
        {
            // do nothing and wait for OnTick().
        }
    }
}

This is not actual code but behavior is same. it should illustrate that this class is not thread safe. :

public class WorkingArea
{
    private List<Worker> _workers;

    public void OnTick()
    {
        foreach(var worker in _workers)
        {
            worker.Start();
        }

        if(_workers.TrueForAll(w => w.Ends))
        {
            PauseTimer();
        }
    }

    public void OnPause() // after timer paused
    {
        foreach(var Worker in _workers)
        {
            worker.Stop();
        }
    }
}
M.kazem Akhgary
  • 18,645
  • 8
  • 57
  • 118
  • 1
    You need to provide a [mcve]. – Enigmativity Dec 08 '16 at 00:47
  • I'm not sure I'm following. It doesn't really matter *who* pauses the timer, there's always a potential for a race condition. However, you've got a lock around the `Tick` method - which should work in any scenario - paused or not. – Rob Dec 08 '16 at 00:47
  • Are you going to be able to provide a [mcve]? – Enigmativity Dec 08 '16 at 01:13
  • @Enigmativity I tried to provide. you see the last class I have is not thread safe. `Worker.Start();` conflicts with `Worker.Stop();` – M.kazem Akhgary Dec 08 '16 at 01:38
  • @M.kazemAkhgary - That's not a [mcve]. I should be able to copy, paste and run your code to see your error or issue. – Enigmativity Dec 08 '16 at 01:55

1 Answers1

0

My timer was already thread safe.

it was all about the fact that I didn't know about Re-entrant locks

So if user from another thread request to pause timer , lock will work just fine and will block until timer is fully paused.

If the timer internally pauses it self it wont deal lock. because its in the same thread the lock was acquired.

public class Timer
{
    private timer = new System.Timers.Timer();

    private bool _guard = false;

    // stops the timer and waits until OnTick returns and lock releases.
    // timer can safely pause it self within OnTick.
    // if user request to pause from another thread, full pause is ensured
    public void Stop()       
    {
        timer.Pause();
        lock (this) // reference of timer. it wont dead lock
        {
            // do nothing and wait for OnTick().
        }
    }

    private void TimerTick(object sender, ElapsedEventArgs e)
    {
        if(_gaurd) return;
        lock (this) // lock per instance
        {
            _gaurd = true;
            if (!_timer.Enabled) return;

            OnTick(); // somewhere inside here timer may pause it self.

            _gaurd = false;
        }
    }
}
Community
  • 1
  • 1
M.kazem Akhgary
  • 18,645
  • 8
  • 57
  • 118