rex
rex

Reputation: 3183

How to make a timer wait for the code in its handler to finish executing as a solution to multi-threading issues

This question follows on from the development of an issue originally posted here. The issue is with editing the properties of an object from multiple-threads.

The context of the application is as follows:

I have a System.Timers.Timer object, which does the below numbered items on every tick. I need this to be done with a timer because I may want to be able to vary the tick interval from 1ms to 60s.

On every tick event:

  1. Run a background worker to read data from a file [order of ms] (or a URL [order of seconds])
  2. When background worker finishes reading data from file, raise an Event (say Ticker_Updated_Event)
  3. In the event handler of Ticker_Updated_Event, run object code which updates a certain property (say this.MyProperty). Additional code is executed in this event handler, including calls to update the GUI [which I believe I have made thread safe (at least i hope)].

Various suggestions have been made including using shared variables and interlocking, but I feel that the quickest(?) and potentially the most robust solution in terms of keeping the rest of my code functional, would be one where I simply force Timer to wait for the code in its tick handler to finish before executing another tick. Can this be as simple as attaching a boolean expression to the start and end of the tick handler?

EDIT 1: I would like the timer to tick as soon as code has finished executing in its handler. I am interested in the performance of my application when running on 1ms tick intervals. I expect the code in the event handler to execute fairly quickly and do not see this being an issue when using it with intervals greater than say 200ms.

EDIT 2: I think I wasn't very clear in setting out the scope of my functionality. I essentially will have 2 states under which my program will run. At any run time, the program will either be:

  1. Using tick intervals >> time needed for execution of code in the handler, which will include doing http requests etc, so no issues here.

OR

  1. Using very short ticks ~1ms to read data from file and run the exact same code as in the first scope, where a loop would easily suffice but mean having to modify the code in the first scope item.

Upvotes: 0

Views: 1899

Answers (3)

Erik Noren
Erik Noren

Reputation: 4339

Going from your previous question and these updates, it sounds like you might want to be aggressive in throwing away ticks. This is based on the comment that the code is too complex to be fully aware of where data changes are happening in multiple threads to make the changes needed to synchronize the use of shared members.

The pseudo code I posted in the previous question which was expanded by ebyrob would be a reasonable choice assuming you haven't kept hidden from us that this application is running multiple times in different application domains!

If you wanted to try to shim something into your existing solution, try something like this:

class UpdateThrottler
{
    static object lockObject = new object();
    static volatile bool isBusyFlag = false;

    static bool CanAcquire()
    {
        if (!isBusyFlag)
            lock(lockObject)
                if (!isBusyFlag) //could have changed by the time we acquired lock
                    return (isBusyFlag = true);
        return false;
    }

    static void Release()
    {
        lock(lockObject)
            isBusyFlag = false;
    }
}

Then in your code, you could do something as simple as:

Tick_Handler(...)
{
    if (UpdateThrottler.CanAcquire())
    {
        try
        {
        //Do your work
        }
        finally
        {
            UpdateThrottler.Release();
        }
    }
}

This has some flexibility in that you can call Release from another location if for some reason you aren't sure to be done working at the end of the Tick handler. This could be because you use a background worker whose callback is used to finish the remaining work or whatever. I'm not entirely sure it's a good practice but short of spending the time to understand your application in its entirety, I'm not sure what else could be done.

At this point we're getting dangerously close to reproducing the (Manual)ResetEvent object.

EDIT: After some discussion I realized it doesn't hurt to have the volatile marker and could ease some potential edge case somewhere.

Upvotes: 1

user645280
user645280

Reputation:

The below will certainly skip ticks, the only other alternative I can think of would amount to an event queue solution. (Which would be more like lock() { push(); } and probably use another thread to do the reading and processing lock() { o=popornull(); } // use o

class TickHandler
{
   static object StaticSyncObject = new object();
   static bool IsBusy = false;

   private TickHandlerObject myHandler;

   static void HandleTimerElapsed(object source, ElapsedEventArgs e)
   {
      lock(StaticSyncObject)
      {
          if( IsBusy ) 
             return;
          else 
             IsBusy = true;
      }
      try {

          // ... do some work here ...  hopefully often faster than interval.

          myHandler.TickerUpdated();
                           // without any IsBusy checks
                           // without thread safety (1 thread at a time calls)
                           // Note: If you're trying to split up threads below
                           //       this, that's up to you.  Or diff question.
                           //       ie: file/URL read vs memory/GUI update

      } finally {
         lock(StaticSyncObject)
         {
            IsBusy = false;
         }
      }
   }
}

PS - Don't necessarily lose heart on the Timer notion. A good timer will have much less clock skew than the average while loop. (I'm assuming clock skew is important here since you're using a timer)

Note: Careful thread safety of memory push from the "data read" code, which could execute under this thread, and proper use of Control.Invoke and/or Control.BeginInvoke should allow you to complete your task without any other "events" needing to be fired.

Performance Note: lock may take around 50 nanoseconds with low contention: http://www.informit.com/guides/content.aspx?g=dotnet&seqNum=600. So, this code should be fine for millisecond resolution. Note: The System.Threading.Interlocked methods may drop some operations to only 6 ns instead of 50 ns, but that difference seems minor in this instance, especially given the complexity of using Interlocked. (see: http://www.dotnetperls.com/interlocked)

Upvotes: 1

rex
rex

Reputation: 3183

I thought I would try and answer my question as well with a solution I have attempted. Please let me know if you see any potential pitfalls in this solution:

This class raises events based on its Timer object executing code:

public class Ticker{

    private TickHandlerObject myHandler; // injected dependency

    //bw is a background worker

    // Handle the Timer tick here:
    private void OnTimedEvent(object source, ElapsedEventArgs e)
    {
        if (myHandler.IsBusy == false) // check here if the handler is still running
        {
            // do work with background worker, that could take time...
        }
    }

    private void bw_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
            // raise event to be handled by myHanlder object
            Updated();
        }

    }
}

This class instaniates the ticker and handles its events.

public class TickHandlerObject{

    // This class subscribes to the Updated event of myTicker.
    private Ticker myTicker;
    myTicker.Updated += this.TickerUpdated;
    public bool IsBusy{get;set;}

    private void TickerUpdated()
    {
        // thread-safing
        this.IsBusy = true;

        // do stuff that can take longer than the tick interval

        //  thread-safing
        this.IsBusy = false;
    }
}

Upvotes: 0

Related Questions