How to guarantee atomicity with state management in a timer

[kirk]

I have a class with a custom event that starts a timer when it's
holding subscriptions and stops the timer otherwise. The timer
repeatedly calls an API which takes a string. For each call the
return value of the API is compared against the last obtained value
from it and if different, I fire my custom event. The string that
the API is using for its input arg is defined by a public property to
this class. My question is, how can I guarantee some level of
atomicity and consistency in the timer function when changes are made
to the string used by the API from the caller? Is there a best
practice, maybe there's a better way to rearchitect this entirely?


// sample code
public class TrackChanges
{
private string LastValue = null;
private string CurrValue = null;

private string szThingToTrack = null;
public string ThingToTrack
{
get { return this.szThingToTrack; }
set { this.szThingToTrack = value; }
}

private void TimerFunction
{
if(this.ThingToTrack == null || this.ChangeOccurred == null)
{ return; }

CurrValue = SomeFunction(this.ThingToTrack);
if(this.CurrValue != this.LastValue)
{
this.LastValue = this.CurrValue;
this.ChangeOccurred(this, EventArgs.Empty);
}
}

public event EventHandler ChangeOccurred
{
add... // Timer.Enabled = true
remove... // Timer.Disabled = false if no other subscriptions
}
}

 

[kirk]

If I understand the question correctly, you are asking how to ensure that  
some code trying to change the ThingToTrack property won't interfere with  
the operation of the TimerFunction.  Of particular concern might be the  
value of ThingToTrack changing from the time you compare it to null and  
the time you call SomeFunction() (other than that, I don't see a  
problem...you have an inherent race condition, but not one that seems  
harmful to me, and it's not one you can fix in any case ).

I think the simplest solution is the same as the one you _should_ have  
applied to the event handler as well.  I would change your TimerFunction 
method to look like this:

    private void TimerFunction()
    {
       string strTrack = ThingToTrack;
       EventHandler handler = ChangeOccurred;

       if(strTrack == null || handler == null)
       {
          return;
       }

       CurrValue = SomeFunction(strTrack);
       if(this.CurrValue != this.LastValue)
       {
          this.LastValue = this.CurrValue;
          handler(this, EventArgs.Empty);
       }
    }

Both the String and Delegate class are immutable, so once you grab the  
reference of the current instance, you can be assured that that instance  
won't change.  Some other thread could in fact change the original source  
 from which you got the instance to point to a different reference, but  
that won't affect your own code, as long as you fix it to look like the  
above.

Note that the above code is still susceptible to other code that tries to  
change both the ThingToTrack property and the ChangeOccurred event  
simultaneously.  Based on what you've written, I don't think that should 
be a problem.  That is, no other code should assume that it can do that. 
But if that's part of your design then yes, you'll need something more  
elaborate.

Pete

Thanks Pete. Good suggestions that I am going to put to use. Other
things I am thinking of to strengthen this are; resetting LastValue to
null if ThingToTrack changes to avoid tracking changes between the old
ThingToTrack and the new ThingToTrack (and) what happens if all event
subscriptions have been pulled somewhere in the middle of
TimerFunction when handler gets called, probably a good idea to wrap
that in a Try/Catch { // do nothing } clause.