0

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?

Improve this question
1
  • Has this been resolved? Commented Sep 19, 2017 at 14:55

2 Answers 2

Reset to default
3

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() { //... } }
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Having goose bumps to see this reply. Thanks for your effort on this. Let me check that
It should be "private async Task DoworkAsync(), Right?
@Raghav, Only if you are using await within that method. I do not know what the method looks like so it would depend on how it is implemented.
This is the minimal change but do note that errors from async void are not observed. You should put a try/catch around the call to DoWorkAsync().
@HenkHolterman async void are allowed on event handlers. I do get your point though. Reference Async/Await - Best Practices in Asynchronous Programming
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.

Improve this answer

4 Comments

You just moved the problem around. Now the call to Start() has become async, you don't know if/how that is solvable.
@HenkHolterman, based on OP's sample after executing Start method, application should run CheckAsync every minute. My example will do exactly same thing in little bid more simply/readable/maintainable way. If OP starts using async in his application he will end up anyway with "problem" to make all methods of application "pipeline" async.
Yes, and that's why this is not complete without a call to Start(). Which the question didn't include either.
@HenkHolterman, Sorry, I think answer is complete for current version of OP's question. Can you suggest improvements to the answer?

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.