I'm not quite sure how to word this, so I'll just paste my code and ask the question:
private void remoteAction_JobStatusUpdated(JobStatus status) {
lock (status) {
status.LastUpdatedTime = DateTime.Now;
doForEachClient(c => c.OnJobStatusUpdated(status));
OnJobStatusUpdated(status);
}
}
private void doForEachClient(Action<IRemoteClient> task) {
lock (clients) {
foreach (KeyValuePair<RemoteClientId, IRemoteClient> entry in clients) {
IRemoteClient clientProxy = entry.Value;
RemoteClientId clientId = entry.Key;
ThreadPool.QueueUserWorkItem(delegate {
try {
task(clientProxy);
#pragma warning disable 168
} catch (CommunicationException ex) {
#pragma warning restore 168
RemoveClient(clientId);
}
});
}
}
}
Assume that any other code wh开发者_JAVA技巧ich modifies the status
object will acquire a lock on it first.
Since the status
object is passed all the way through to multiple ThreadPool
threads, and the call to ThreadPool.QueueUserWorkItem
will complete before the actual tasks complete, am I ensuring that the same status
object gets sent to all clients?
Put another way, when does the lock (status)
statement "expire" or cause its lock to be released?
Locks don't expire. When a thread tries to pass the lock
statement it can only do it if no other thread is executing inside a lock
block having a lock on that particular object instance used in the lock
statemement.
In your case it seems that you have a main thread executing. It will lock both the status
and the clients
instances before it spins of new tasks that are executed on seperate threads. If any code in the new threads want to acquire a lock on either status
or clients
it will have to wait until the main thread has released both locks by leaving both lock
blocks. That happens when remoteAction_JobStatusUpdated
returns.
You pass the status
object to each worker thread and they are all free to do whatever they want to do with that object. The statement lock (status)
in no way protects the status
instance. However, if any of the threads tries to execute lock (status)
they will block until the main thread releases the lock.
Using two separate object instances to lock can lead to deadlock. Assume one thread executes the following code:
lock (status) {
...
lock (clients) {
...
}
}
Another thread executes the following code where the locks are acquired in the reverse sequence:
lock (clients) {
...
lock (status) {
...
}
}
If the first thread manages to get the status first and the second the clients lock first they are deadlocked and both threads will no longer run.
In general I would advice you to encapsulate your shared state in a separate class and make access to it thread safe:
class State {
readonly Object locker = new Object();
public void ModifyState() {
lock (this.locker) {
...
}
}
public String AccessState() {
lock (this.locker) {
...
return ...
}
}
}
You can also mark you methods with the [MethodImpl(MethodImpl.Synchronized)] attribute, but it has its pitfalls as it will surround the method with a lock (this)
which in general isn't recommended.
If you want to better understand what is going on behind the scenes of the lock
statement you can read the Safe Thread Synchronization article in MSDN Magazine.
The locks certainly don't "expire" on their own, the lock will be valid until the closing brace of the lock(..){} statement.
精彩评论