Will the threadpool queue a timer's callback function, sometimes scheduling more than one thread at the same time?

Question

In the following code TimerRecalcStatisticsElapsed should only have one instance of it running. The worker methods that this callback invokes is made to run in sequence, with a maximum of one thread running at a time.

Question Part 1:

If the timer's callback runs an a threadpool thread (as opposed to running the callback on a separate thread), is it correct to say the the threadpool might queue and defer the thread for later execution based on conditions (MaxThreads reached, threadpool internal logic)?

Question Part 2:

Assuming it's possible for one timer callback to be queued for anything but immediate execution, does that mean that any number of thread callbacks may execute concurrently?

Question Part 3

Assuming part 2 is true, does that mean the code below can ever have more than one callback operating at the same time?

The reason I'm asking is because there are several thousand instances of this class running on a multi-CPU server. I'm also seeing data corruption consistent with an out-of-order operation of // Do Work Here.

Aside

// Do work here internally works with a System.Collections.Dictionary and edits the values of y. It also removes some keys for a subsequent function that is called serially. That function is missing keys (x) that were previously present in the first call. I think this is because there is a race condition with the final statement obj.cleanupdata()

public class SystemTimerTest
   {

    readonly System.Timers.Timer timerRecalcStatistics;
    readonly System.Diagnostics.Stopwatch stopwatchForRecalcStatistics = new System.Diagnostics.Stopwatch();


    public SystemTimerTest(TimeSpan range, DataOverwriteAction action)
    {
        int recalculateStatisticsEveryXMillseconds = 1000;

        timerRecalcStatistics = new System.Timers.Timer(recalculateStatisticsEveryXMillseconds);
        timerRecalcStatistics.AutoReset = true;
        timerRecalcStatistics.Elapsed += new System.Timers.ElapsedEventHandler(TimerRecalcStatisticsElapsed);
        timerRecalcStatistics.Interval = recalculateStatisticsEveryXMillseconds;
        timerRecalcStatistics.Enabled = true;


        this.maxRange = range;
        this.hashRunningTotalDB = new HashRunningTotalDB(action);
        this.hashesByDate = new HashesByDate(action);
        this.dataOverwriteAction = action;
    }


    private void TimerRecalcStatisticsElapsed(object source, System.Timers.ElapsedEventArgs e)
    {
        stopwatchForRecalcStatistics.Start();
        Console.WriteLine("The TimerRecalcStatisticsElapsed event was raised at {0}", e.SignalTime.ToString("o"));

         // DO WORK HERE


        stopwatchForRecalcStatistics.Stop();
        double timeBuffer  = GetInterval(IntervalTypeEnum.NearestSecond, e.SignalTime) - stopwatchForRecalcStatistics.ElapsedMilliseconds;

        if (timeBuffer > 0)
            timerRecalcStatistics.Interval = timeBuffer;
        else
            timerRecalcStatistics.Interval = 1;

        stopwatchForRecalcStatistics.Reset();         
        timerRecalcStatistics.Enabled = true;
    }
 }

Abreu

I think the problem is that the TimerRecalcStatisticsElapsed method is taking over 1 second to run and therefore multiple instances could be executing at the same time. – Tennyson

@Tennyson I was hoping to avoid this by setting Autoreset to false, running the task, and re-enabling the timer once the event complete. Is this a good solution, or is there something more robust? – Abreu

Answer 1

ad 1) It is not important whether ThreadPool can defer execution of callback method, because anyway callback is not guaranteed to complete execution before another timer interval(s) elapses (thread can be suspended by thread scheduler for example, or callback might call long-running function).

ad 2) This is what MSDN says about Timer class:

If the SynchronizingObject property is null, the Elapsed event is raised on a ThreadPool thread. If processing of the Elapsed event lasts longer than Interval, the event might be raised again on another ThreadPool thread. In this situation, the event handler should be reentrant.

So the answer is YES, callback can be executing on multiple threads concurrently.

ad 3) YES. And you should either avoid using shared resources (timerRecalcStatistics, stopwatchForRecalcStatistics) in callback method, or synchronize access to these shared resources (for example with lock), or set appropriate object to Timer's SynchronizingObject property, or set AutoReset property of Timer to false (and enable timer again at the end of timer callback).

UPDATE: I thing that Jon Skeet's answer doesn't solve your problem. Also implementing your own SynchonizingObject is IMHO more complicated than necessary (but it's hard to say without knowing whole problem). I hope this implementation should work (but I didn't tested it):

public class MySynchronizeInvoke : ISynchronizeInvoke
{
    private object SyncObject = new Object();
    private delegate object InvokeDelegate(Delegate method, object[] args);

    public IAsyncResult BeginInvoke(Delegate method, object[] args)
    {
        ElapsedEventHandler handler = (ElapsedEventHandler)method;
        InvokeDelegate D = Invoke;
        return D.BeginInvoke(handler, args, CallbackMethod, null);
    }

    private void CallbackMethod(IAsyncResult ar)
    {
        AsyncResult result = ar as AsyncResult;
        if(result != null)
            ((InvokeDelegate)result.AsyncDelegate).EndInvoke(ar);
    }

    public object EndInvoke(IAsyncResult result)
    {
        result.AsyncWaitHandle.WaitOne();
        return null;
    }

    public object Invoke(Delegate method, object[] args)
    {
        lock(SyncObject)
        {
            ElapsedEventHandler handler = (ElapsedEventHandler)method;
            handler(args[0], (ElapsedEventArgs)args[1]);
            return null;
        }
    }

    public bool InvokeRequired
    {
        get { return true; }
    }
}

Plerre

Thank you.. do you know how I can implement SynchronizingObject on my other thread? Perhaps I can use John Skeet's answer but I don't know how to use this utility class: https://mcmap.net/q/910688/-synchronizingobject-for-an-event – Abreu

Answer 2

From the documentation on System.Timers.Timer:

If the SynchronizingObject property is null, the Elapsed event is raised on a ThreadPool thread. If processing of the Elapsed event lasts longer than Interval, the event might be raised again on another ThreadPool thread. In this situation, the event handler should be reentrant.

So to answer your questions:

1. Yes, it runs on a threadpool thread, and is subject to threadpool filling up and deferring like anything else. Given that the threadpool now has a maximum of hundreds of threads, this shouldn't be an issue. If it is, you have bigger problems.

2. Assuming that you do not set a synchronizing object or otherwise sychronize your callback, yes, multiple callback can overlap. If you give the timer a synchronizing object, it will not 'overlap' events.

3. The code that you provided does not synchronize it's callback in any way, and so yes it can have multiple overlapping, simultaneously executing copies of your callback. You should synchronize the method using something like a lock statement if you want all of the instances of the class to be synchronized one another, or use the SynchronizingObject of the timer if you want each individual instance of the class to only ever have one callback running at any given time.

Martelli

Thank you.. do you know how I can implement SynchronizingObject on my other thread? Perhaps I can use John Skeet's answer but I don't know how to use this utility class: https://mcmap.net/q/910688/-synchronizingobject-for-an-event – Abreu

I am not sure what you mean by "implement SynchronizingObject on my other thread." You don't implement SynchronizingObject- it's a property on the timer. As long as it isn't null, that timer will not overlap events. – Martelli