async await not working with Timer

Question

I have a Presence monitor class which is used to detect users active/inactive status. That class has a timer in its Start method which called on application start:

public class PresenceMonitor
{
    private volatile bool _running;
    private Timer _timer;
    private readonly TimeSpan _presenceCheckInterval = TimeSpan.FromMinutes(1);


    public PresenceMonitor()
    {
    }

    public void Start()
    {
        // Start the timer
        _timer = new Timer(_ =>
        {
            Check();
        }, null, TimeSpan.Zero, _presenceCheckInterval);
    }

    private void Check()
    {
        if (_running)
        {
            return;
        }

        _running = true;

        // Dowork
    }
}

The "Check" method is fired after every one minute. That piece of code is working fine but now my "Do work" methods have become async await so I had to change this Presence Monitor class to something like this:

public class PresenceMonitor
{
    private volatile bool _running;
    private Timer _timer;
    private readonly TimeSpan _presenceCheckInterval = TimeSpan.FromMinutes(1);


    public PresenceMonitor()
    {
    }

    public void Start()
    {
        // Start the timer
        var timer = new System.Threading.Timer(async (e) =>
        {
            await CheckAsync();
        }, null, TimeSpan.Zero, _presenceCheckInterval);
    }

    private async Task CheckAsync()
    {
        if (_running)
        {
            return;
        }

        _running = true;

        // await DoworkAsync
    }
} 

Unfortunately "CheckAsync" method now is getting fired once only instead of every minute. Can you tell me what I am doing wrong here to call async await after regular intervals?

Is there any correct way to do the same?

Answer 1

If I understand correctly your requirements, you can get rid of timer and use asynchronous loop.
But you need make Start method asynchronous too

public class PresenceMonitor
{
    private volatile bool _running; // possible not needed "volatile" anymore
    private readonly int _presenceCheckInterval = 60000; // Milliseconds

    public PresenceMonitor()
    {
    }

    public async Task Start()
    {
        while (true) // may be use some "exit" logic
        {
           await CheckAsync();
           await Task.Delay(_presenceCheckInterval) 
        }
    }

    private async Task CheckAsync()
    {
        if (_running)
        {
            return;
        }

        _running = true;

        // await DoworkAsync
    }
} 

Then you can start monitoring

var monitor = new PresenceMonitor();
await monitor.Start();

You can even start monitoring in synchronous way

var monitor = new PresenceMonitor();
monitor.Start(); // Will start monitoring

But approach above is "dangerous" in the way, that any exception thrown inside CheckAsync method will not be propagated. When you start using async-await be ready to "convert" whole application to support it.

Answer 2

You could consider creating an event and handler to handle the timer ticks and then invoke your check.

public class PresenceMonitor {
    private volatile bool _running;
    private Timer timer;
    private readonly TimeSpan _presenceCheckInterval = TimeSpan.FromMinutes(1);

    public PresenceMonitor() {
        Tick += OnTick;
    }

    public void Start() {
        if (_running) {
            return; //already running
        }
        // Start the timer
        timer = new System.Threading.Timer(_ => {
            Tick(this, EventArgs.Empty);//rasie event
        }, null, TimeSpan.Zero, _presenceCheckInterval);
    }

    private event EventHandler Tick = delegate { };
    private async void OnTick(object sender, EventArgs args) {
        if (_running) {
            return;
        }
        _running = true;
        await DoworkAsync();
    }

    private Task DoworkAsync() {
        //...
    }
}