Locking on field or local variable?
I believe
object o = new Object();
should be outside the method as aField
.Since each thread is getting a new instance of
o
, there will be multiple locks.What am I missing here? Shouldn't it lock on fields in this specific case?
Your understanding is correct. The code is broken. In this implementation even though lock will be active, it will not provide synchronization as it will be on different objects.
From Microsoft Docs
When you synchronize thread access to a shared resource, lock on a dedicated object instance (for example, private readonly object balanceLock = new object();) or another instance that is unlikely to be used as a lock object by unrelated parts of the code. Avoid using the same lock object instance for different shared resources, as it might result in deadlock or lock contention. In particular, avoid using the following as lock objects:
this, as it might be used by the callers as a lock. Type instances, as those might be obtained by the typeof operator or reflection. string instances, including string literals, as those might be interned. Hold a lock for as short time as possible to reduce lock contention.
Yes. It is broken.
You want a static readonly object as a private field to lock on. As you suspect, your example code creates a new object every time you call Do, and hence the lock will have nothing to hold onto and won't work at all.
private static object syncRoot = new object();
lock (syncRoot) { }