Single-threaded timer

Question

I've created a System.Threading.Timer wrapper in C#. The tasks to be triggered when the timer elapses have highly variable execution times. My design criteria are:

1. Provide a strongly typed timer state in the callback
2. Run the timer callback on a periodic basis
3. If the timer's period elapses before the timer callback completes
   1. Do not run the timer callback a second time in parallel
   2. Run time timer callback again immediately after the running callback completes
4. Do not run the timer callback more than once per period

As an example, given a timer that elapses every 3 seconds: 3s - Timer elapse triggers task 6s - Timer elapse triggers task, but task does not run because
the previous run is not complete 9s - Timer elapse triggers task, but task does not run because
the previous run is not complete 10s - Task finishes running 10s - Task immediately runs again because the previous run crossed
a period boundary 11s - Task finishes running 12s - Timer elapse triggers task

I ended up with the following class:

public class NonOverrunningTimer<TState>
{
    private readonly Func<TState, TState> _delegate;
    private readonly TimeSpan _interval;
    private readonly System.Threading.Timer _timer;

    public NonOverrunningTimer(Func<TState, TState> @delegate, TimeSpan interval, TState initialState = default(TState))
    {
        if (interval == TimeSpan.Zero)
        {
            throw new Exception($"Cannot initialize a timer with a period of {TimeSpan.Zero}");
        }

        State = initialState;
        _delegate = @delegate;
        _interval = interval;
        _timer = new System.Threading.Timer(UntypedCallback, null, TimeSpan.Zero, interval);
    }

    public TState State { get; private set; }

    private volatile bool _running;
    private volatile bool _waiting;
    private readonly object _executeLock = new object();
    public void UntypedCallback(object state)
    {
        if (_running && _waiting)
        {
            return;
        }

        if (_waiting && !_running)
        {
            _waiting = false;
        }

        if (_running)
        {
            _waiting = true;
            return;
        }

        using (var @lock = new TryLock(_executeLock))
        {
            if ([email protected])
            {
                return;
            }

            _running = true;

            State = _delegate(State);

            _running = false;
            if (_waiting)
            {
                _waiting = false;
                _timer.Change(TimeSpan.Zero, _interval);
            }
        }
    }

    private class TryLock : IDisposable
    {
        private object _locked;

        public bool HasLock { get; private set; }

        public TryLock(object obj)
        {
            if (Monitor.TryEnter(obj))
            {
                HasLock = true;
                _locked = obj;
            }
        }

        public void Dispose()
        {
            if (!HasLock)
            {
                return;
            }

            Monitor.Exit(_locked);
            _locked = null;
            HasLock = false;
        }
    }
}

I'm interested in general feedback as well as whether something like this already exists in the BCL/Framework libraries and I just missed it.

Answer 1

The TryLock class shouldn't be netested.

I'd also add a factory method to it and make it much simpler like this and create an instance only when a lock could be accuired:

private class Locker : IDisposable
{
    private object _locked;

    private Locker(object obj)
    {       
        _locked = obj;
    }

    public static Locker Create(object obj)
    {
        return Monitor.TryEnter(obj) ? new Locker(obj) : null;
    }

    public void Dispose()
    {
        Monitor.Exit(_locked);
    }
}

Later instead of checking the property you check if the instance isn't null:

using (var tryLock = Locker.Create(_executeLock))
{
    if (tryLock == null)
    {
        return;
    }

    ...
}

---

@variable

This kind of naming can and should really be avoided. I'm sure you can come up with a better name then just a delegate which says pretty nothing about the method.