Sharing data while multithreading - are non-static variables ok?
In Joseph Albahari's famous 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 with a mechanism lock()
in all read / write locations and the job is done.
My question is about implementing a property of a class. I understand that this is valid if I implement (for example) the property Timeout
with a backup store static
:
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 the variable _Timeout
common to all instances of the class.
But in my case, multithreading is private to the class instance. It starts with New()
and ends with Dispose()
. Both main and worker stream access the Timeout
property (but _Timeout
the backup store is never accessible outside of the property getter / setter).
I don't want the meaning to _Timeout
be generally accepted. I want it to be unique for every instance of the class.
My question is, can I safely remove the variable static
from _Timeout
for this?
Note. Sorry if there are any errors in the code, I am actually using VB.NET and I converted it with a tool. Hope the main question remains clear.
source to share
Absolutely safe and fairly recommended (static variables are painful to test, even if necessary). Assuming that you are safe, you also mean that you must not forget that:
this.Timeout = 0; // This is safe and valid
++this.Timeout; // This is safe but not valid
Because the operator is ++
not atomic (which is why we have a class Interlocked
). Of course, in this situation:
if (this.Timeout == 0)
Timeout = 10;
Because even if every access is safe (I would say that reading a property is always safe, but you can read the old value without a barrier lock
), this is not an atomic operation, and the value can change after the test and before the new assignment. More difficult?
if (this.Timeout == 0)
Timeout = Timeout * 2;
In this case, every time you read Timeout
, you might get a different value. For this reason, I said that locking inside a property is rarely useful, unless the property is read-only. It is much better to remove this lock from the get / set property and wrap your code in a lock statement:
lock (_locker) {
if (this.Timeout == 0)
Timeout = Timeout * 2;
}
Also note that for int _Timeout
(I'm assuming assigning with false
is just a typo), you can just remove the lock and do it volatile
:
private volatile int _Timeout;
Of course, this won't solve the other problems described, but it can be useful (and faster) for a read-only property (or for fairly controlled situations, modifiers volatile
can be tricky and have a different meaning from C and it's easy to forget that accessing them is atomic, but no more).
source to share
A static keyword is an orthogonal (ie: independent or unrelated) stream.
The static keyword should only be used if only one instance of this property is required / field / class / method / etc.
In your situation, you don't need static in _Timeout if you want this value to be unique to the instance.
I haven't read the article you are linking to, but I think you may have misunderstood it (or the author misunderstood the use of static)