In well-known Joseph Albahari's article on Threading, all class variables used by multiple threads are declared as static
fields => all threads have access to the same variable. This needs to be equipped by lock()
mechanism at all places of reading/writing and the job is done.
My question is about class property implementation. I understand it is valid if I implement (for example) property Timeout
using static
backing store:
class MyClassWithWorkerThread {
private readonly object _locker = new object();
private static int _Timeout = false;
private int Timeout {
get {
lock (_locker) {
return _Timeout;
}
}
set {
lock (_locker) {
_Timeout = value;
}
}
}
}
This will make variable _Timeout
shared among all class instances.
But in my case, multi-thread processing is private to class instance. It starts with New()
and ends with Dispose()
. Both main thread and worker thread access Timeout
property (but _Timeout
backing store is never accessed outside the property getter/setter).
I do not want _Timeout
value to be application-wide. I want to have it unique for every class instance.
My question is: may I safely remove static
from _Timeout
variable in order to achieve this?
Note: I'm sorry if there are any errors in the code, I'm actually using VB.NET and I converted it using a tool. I hope the main question is still clear.
Absolutely safe and pretty recommended (static variables are painful for testing even when needed). Assuming with safe you also mean valid what you don't have to forget is that:
this.Timeout = 0; // This is safe and valid
++this.Timeout; // This is safe but not valid
Because ++
operator is not atomic (that's why we have Interlocked
class). Of course same apply in this situation:
if (this.Timeout == 0)
Timeout = 10;
Because even if each access is safe (I would say reading a property is always safe but you may read an old value without lock
barrier) it's not an atomic operation and value may change after test and before new assignment. Even more complicate?
if (this.Timeout == 0)
Timeout = Timeout * 2;
In this case each time you read Timeout
you may get a different value. For this reason I said lock inside the property is seldom useful unless it's a read-only property. Much better to remove that lock from property get/set and wrap your code in a lock statement:
lock (_locker) {
if (this.Timeout == 0)
Timeout = Timeout * 2;
}
Also note that for int _Timeout
(I assume assignment with false
is just a typo) you may simply remove lock and make it volatile
:
private volatile int _Timeout;
Of course this won't solve the other described problems but it may be useful (and faster) for a read-only property (or for pretty controlled situations, volatile
modifiers can be tricky and it has a different meaning compared with C and it's easy to forget that to access them is atomic but nothing more).