Synchronising access to state variable which is reset by a timer

Question

I am currently working on a project in C#. I am syncing access to a state variable using a single lock. This state variable is triggered to be set for a given period of time and then should have its value reset. My current code is as follows.

using System.Threading;

class Test
{
  object syncObj = new object();
  bool state = false;
  Timer stateTimer;

  Test()
  {
    stateTimer = new Timer(ResetState, this, Timeout.Infinite, Timeout.Infinite);
  }

  void SetState()
  {
    lock(syncObj)
    {
      state = true;
      stateTimer.Change(1000, Timeout.Infinite);    
    }
  }

  static void ResetState(object o)
  {
    Test t = o as Test;
    lock(t.syncObj)
    {
      t.state = false;
    }
  }  
}

Given that it is valid to call SetState again before ResetState is called by the Timer (i.e. it is allowed to extend the period of time that state is true), I can imagine situations where a single lock may not be enough. The specific case I'm thinking of is this

Both SetState and ResetState are entered at the same time, on the main thread and the Timer thread respectively
SetState acquires the lock first and correctly sets state to true and triggers the timer to start again
ResetState then incorrectly sets state to false meaning that state is not true for the expected period of time

I've been scratching my head over this one for a little while. The closest I got to being able to solve it was by using two locks but in the end I found this caused other issues (at least, the way I'd done it).

Is there a known way to solve this problem (and should I be reading something to refresh my knowledge of synchronisation)?

UPDATE: I forgot to mention that the current state of the timer cannot be queried in this instance. If it could I would imagine checking the remaining time in ResetState to determine that the timer is really stopped.

score 3 · Accepted Answer · edited May 23 '17 at 12:04

First and foremost: it's a bad idea to expose the locking object publicly!

class Test
{
  private object syncObj = new object();
  private bool state = false;
  private Timer stateTimer;

  public Test()
  {
    stateTimer = new Timer(ResetState, this, Timeout.Infinite, Timeout.Infinite);
  }

  public void SetState()
  {
    lock(syncObj)
    {
      state = true;
      stateTimer.Change(1000, Timeout.Infinite);    
    }
  }

  public static void ResetState(object o)
  {
    Test t = o as Test;
    t.ResetState();
  }

Since you're no longer exposing the locking object, you'll have to create another method to reset the state:

  public void ResetState()
  {
    lock(syncObj)
    {
      state = false;
      stateTimer.Change(Timeout.Infinite, Timeout.Infinite);
    }
  }  


}

Note that we also take care of another problem in the new ResetState method and that is to force the timer not to fire again. This will only guarantee that the state flag will not be in out of sync with the timer; i.e. if you set the state, it will remain set for the expected amount of time or until the reset method is called.

Update

If you want to reject the reset attempt, then make the state variable an enum:

enum EState
{
    Off = 0,
    On = 1,
    Waiting = 2
}

private EState state = EState.Off;

// Provide a state property to check if the state is on or of (waiting is considered to be Off)
public bool State{ get{ return state == EState.On;} }

In addition, you will now need to modify the SetState method and you will need two reset methods (the private one will be used with by the timer).

public void SetState()
{
    lock(syncObj)
    {
        state = EState.Waiting;
        stateTimer.Change(1000, Timeout.Infinite);
    }
}

public void ResetState()
{
    lock(syncObj)
    {
        if(state != EState.Waiting)
        {
            state = EState.Off;
        }
    }
}

private void TimerResetState()
{
    lock(syncObj)
    {
        state = EState.Off;
        stateTimer.Change(Timeout.Infinite, Timeout.Infinite);
    }
}

So now your constructor will look like this:

public Test()
{
    stateTimer = new Timer(TimerResetState, this, Timeout.Infinite, Timeout.Infinite);
}

Things should work roughly along those lines.

P.S. Note that I assume the correct behavior is to reset the timer, but if your goal is to reject reset attempts until the timer fires, then this can be achieved too (although it's just a bit more difficult). — Kiril, Nov 26 '12 at 19:53
Thanks for the comments Lirik. I completely agree it's a bad idea to expose the locking object publicly - that wasn't my intention but I didn't make that clear in my code. My goal is indeed to reject reset attempts until the timer fires. Any ideas on how this could be achieved in this model? — Andrew, Nov 27 '12 at 08:53
@Andrew I updated my answer, please see the update for more info. — Kiril, Nov 27 '12 at 15:01

Synchronising access to state variable which is reset by a timer

1 Answers1

Update