MQTTnet icon indicating copy to clipboard operation
MQTTnet copied to clipboard

Improve async lock

Open chkr1011 opened this issue 3 years ago • 4 comments

This pull request adds a new implementation of the Async Lock which has no longer a dead lock issue.

chkr1011 avatar Sep 16 '22 21:09 chkr1011

Should the _isDisposed check in WaitAsync move inside the _syncRoot lock a bit further down?

logicaloud avatar Sep 17 '22 05:09 logicaloud

Should the _isDisposed check in WaitAsync move inside the _syncRoot lock a bit further down?

Why do you think so? The field is atomic to there is no need to lock first in my opinion.

chkr1011 avatar Sep 17 '22 09:09 chkr1011

I was thinking about multithreaded access where the bool is modified in one thread, then sits in a register and another thread reads a stale value. Maybe the bool could be declared as volatile or moved into the lock (https://learn.microsoft.com/en-us/archive/msdn-magazine/2012/december/csharp-the-csharp-memory-model-in-theory-and-practice#thread-communication-patterns). I'm not sure if it would make much of a difference in practice.

logicaloud avatar Sep 17 '22 10:09 logicaloud

Makes sense. I will move it for sure.

chkr1011 avatar Sep 17 '22 13:09 chkr1011