I have a few objects that I'd like to send to the server, but I want to make sure that this is the only thread 开发者_如何学Gothat moving the data from Stage to Upload. Is the following code valid in a multithreaded environment?
List<CounterInternal> UploadToServer = new List<CounterInternal>();
List<CounterInternal> StagingQueue = new List<CounterInternal>();
lock (this.UploadToServer)
lock (this.StagingQueue)
{
if (UploadToServer.Count == 0)
{
UploadToServer = StagingQueue.DoDeepCopyExtensionMethod();
// is the following line valid given that I have a Lock() on it?
StagingQueue = new List<CounterInternal>();
}
}
}
Technically, yes, but this is a bad idea. Consider this C# source file:
using System;
class Foo {
static object foo = new object();
static void Main() {
lock (foo) {
foo = new object();
}
}
}
The Main()
method will compile to:
.method private static hidebysig
default void Main () cil managed
{
// Method begins at RVA 0x2100
.entrypoint
// Code size 35 (0x23)
.maxstack 3
.locals init (
object V_0)
IL_0000: ldsfld object Foo::foo
IL_0005: stloc.0
IL_0006: ldloc.0
IL_0007: call void class [mscorlib]System.Threading.Monitor::Enter(object)
.try { // 0
IL_000c: newobj instance void object::'.ctor'()
IL_0011: stsfld object Foo::foo
IL_0016: leave IL_0022
} // end .try 0
finally { // 0
IL_001b: ldloc.0
IL_001c: call void class [mscorlib]System.Threading.Monitor::Exit(object)
IL_0021: endfinally
} // end handler 0
IL_0022: ret
} // end of method Foo::Main
This corresponds to the following source (decompiled by hand):
static void Main() {
object V_0 = foo;
Monitor.Enter(V_0);
try {
foo = new object();
} finally {
Monitor.Exit(V_0);
}
}
So the object that is locked on will be stored in a local -- this guarantees that the object's monitor will be released even if the object reference stored in the field is replaced. There will be no deadlocks created by this technique alone, and any other threads already blocked on Monitor.Enter()
will continue to block as usual, until this thread releases the lock.
However, any thread that enters this method after you have reassigned the object but before the active thread releases the lock will be acquiring a lock on the new object and hence there can be two threads in the lock block at the same time.
A better solution would be to use a separate object and lock on it instead. I usually use something of the class System.Object
(or just object
) since all it is doing is acting as a mutex. This will allow all threads to lock on the same object, while allowing the other object reference to change. This is also a useful technique when you need to lock to mutate a value type, which cannot be locked on.
It's not a good idea. lock
operates on a specific instance and you create a new one when you do a new List<>
. Another thread could come along and lock the new StagingQueue while another thread is still within what you thought was the locked region.
You should either:
lock
on another instance variable (preferably areadonly
one), maybe even anObject
instance that exists purely to be the object that is locked.- Modify the algorithm so that it does not create a new instance of
List<>
. If all you want to do is empty the list, for instance, just callStagingQueue.Clear()
.
精彩评论