C# manual lock/unlock
Solution 1
The lock
keyword is just syntactic sugar for Monitor.Enter and Monitor.Exit:
Monitor.Enter(o);
try
{
//put your code here
}
finally
{
Monitor.Exit(o);
}
is the same as
lock(o)
{
//put your code here
}
Solution 2
Thomas suggests double-checked locking in his answer. This is problematic. First off, you should not use low-lock techniques unless you have demonstrated that you have a real performance problem that is solved by the low-lock technique. Low-lock techniques are insanely difficult to get right.
Second, it is problematic because we don't know what "_DoSomething" does or what consequences of its actions we are going to rely on.
Third, as I pointed out in a comment above, it seems crazy to return that the _DoSomething is "done" when another thread is in fact still in the process of doing it. I don't understand why you have that requirement, and I'm going to assume that it is a mistake. The problems with this pattern still exist even if we set "done" after "_DoSomething" does its thing.
Consider the following:
class MyClass
{
readonly object locker = new object();
bool done = false;
public void DoSomething()
{
if (!done)
{
lock(locker)
{
if(!done)
{
ReallyDoSomething();
done = true;
}
}
}
}
int x;
void ReallyDoSomething()
{
x = 123;
}
void DoIt()
{
DoSomething();
int y = x;
Debug.Assert(y == 123); // Can this fire?
}
Is this threadsafe in all possible implementations of C#? I don't think it is. Remember, non-volatile reads may be moved around in time by the processor cache. The C# language guarantees that volatile reads are consistently ordered with respect to critical execution points like locks, and it guarantees that non-volatile reads are consistent within a single thread of execution, but it does not guarantee that non-volatile reads are consistent in any way across threads of execution.
Let's look at an example.
Suppose there are two threads, Alpha and Bravo. Both call DoIt on a fresh instance of MyClass. What happens?
On thread Bravo, the processor cache happens to do a (non-volatile!) fetch of the memory location for x, which contains zero. "done" happens to be on a different page of memory which is not fetched into the cache quite yet.
On thread Alpha at the "same time" on a different processor DoIt calls DoSomething. Thread Alpha now runs everything in there. When thread Alpha is done its work, done is true and x is 123 on Alpha's processor. Thread Alpha's processor flushes those facts back out to main memory.
Thread bravo now runs DoSomething. It reads the page of main memory containing "done" into the processor cache and sees that it is true.
So now "done" is true, but "x" is still zero in the processor cache for thread Bravo. Thread Bravo is not required to invalidate the portion of the cache that contains "x" being zero because on thread Bravo neither the read of "done" nor the read of "x" were volatile reads.
The proposed version of double-checked locking is not actually double-checked locking at all. When you change the double-checked locking pattern you need to start over again from scratch and re-analyze everything.
The way to make this version of the pattern correct is to make at least the first read of "done" into a volatile read. Then the read of "x" will not be permitted to move "ahead" of the volatile read to "done".
Solution 3
You can check the value of done
before and after the lock:
if (!done)
{
lock(this)
{
if(!done)
{
done = true;
_DoSomething();
}
}
}
This way you won't enter the lock if done
is true. The second check inside the lock is to cope with race conditions if two threads enter the first if
at the same time.
BTW, you shouldn't lock on this
, because it can cause deadlocks. Lock on a private field instead (like private readonly object _syncLock = new object()
)
Solution 4
The lock
keyword is just syntactic sugar for the Monitor
class. Also you could call Monitor.Enter()
, Monitor.Exit()
.
But the Monitor class itself has also the functions TryEnter()
and Wait()
which could help in your situation.
Solution 5
I know this answer comes several years late, but none of the current answers seem to address your actual scenario, which only became apparent after your comment:
The other threads don't need to use any information generated by ReallyDoSomething.
If the other threads don't need to wait for the operation to complete, the second code snippet in your question would work fine. You can optimize it further by eliminating your lock entirely and using an atomic operation instead:
private int done = 0;
public void DoSomething()
{
if (Interlocked.Exchange(ref done, 1) == 0) // only evaluates to true ONCE
_DoSomething();
}
Furthermore, if your _DoSomething()
is a fire-and-forget operation, then you might not even need the first thread to wait for it, allowing it to run asynchronously in a task on the thread pool:
int done = 0;
public void DoSomething()
{
if (Interlocked.Exchange(ref done, 1) == 0)
Task.Factory.StartNew(_DoSomething);
}
Related videos on Youtube
Daniel
Updated on July 09, 2022Comments
-
Daniel almost 2 years
I have a function in C# that can be called multiple times from multiple threads and I want it to be done only once so I thought about this:
class MyClass { bool done = false; public void DoSomething() { lock(this) if(!done) { done = true; _DoSomething(); } } }
The problem is
_DoSomething
takes a long time and I don't want many threads to wait on it when they can just see thatdone
is true.
Something like this can be a workaround:class MyClass { bool done = false; public void DoSomething() { bool doIt = false; lock(this) if(!done) doIt = done = true; if(doIt) _DoSomething(); } }
But just doing the locking and unlocking manually will be much better.
How can I manually lock and unlock just like thelock(object)
does? I need it to use same interface aslock
so that this manual way andlock
will block each other (for more complex cases).-
Jethro about 13 yearsCan I ask what _DoSomething() does?
-
Cody Gray about 13 yearsConsider the carefully the disavantages of using
lock(this)
: Why is lock(this) {...} bad? -
Eric Lippert about 13 yearsI am confused. If multiple threads are waiting a long time on _DoSomething, isn't that what you want? If two threads both say "synchronously ensure that _DoSomething has been called" at the same time then they both have to wait until at least one of them does it, no? Why is this a problem? Waiting until its done sounds like the desired feature. Can you explain?
-
Eric Lippert about 13 yearsAlso, can you explain why you believe that locking and unlocking explicitly with the monitors is better than the lock statement?
-
Daniel about 13 yearsI don't want other threads to wait until its done, I want other threads make sure its initiated.
-
Greg D about 13 years@Dani: What are the other threads supposed to do if it isn't done, continue executing as though it is? That's broken, because then it isn't initialized and they're executing as though it were.
-
Eric Lippert about 13 years@Greg: It depends on what "DoSomething" actually does. For example, suppose "DoSomething" is "send an email to the administrator when the service starts up". That's something that you only want done once. If ten threads all try to send email at the same time, nine of them can immediately return if the tenth is currently working on the problem; the assumption here is that "working on it" is enough; you don't have to wait around for it to finish. Since the original poster has not told us what sort of thing DoSomething is doing, it is really hard to give a correct analysis of the code.
-
-
Groo about 13 yearsNote also that double-checked locking is broken in some hardware/software architectures. In CLR, it is implemented correctly and allows for such usage.
-
Brian Gideon about 13 yearsLooks good. One caveat...
done
needs to be marked asvolatile
. -
LukeH about 13 years
lock
creates a memory barrier, so skipping thevolatile
won't cause weird errors, only a possible performance hit: Withoutvolatile
the code might enter the outerif (!done)
section and contest the lock when it doesn't really need to. Once the lock has been acquired then the code will see the up-to-date value ofdone
in the innerif (!done)
check. -
Thomas Levesque about 13 years@LukeH, that's what I thought, but I wasn't sure... thanks for the clarification.
-
Eric Lippert about 13 yearsNote that in C# 4 we changed the codegen slightly so that the enter goes inside the try block. See blogs.msdn.com/b/ericlippert/archive/2009/03/06/… for the messy details.
-
Eric Lippert about 13 years@Groo, @LukeH, @Thomas: I do not believe any of your analyses are correct. @Brian: done does not need to be marked as volatile, though that solves the problem. What needs to happen is the first read of done must be performed with volatile semantics. It is not necessary for the second read to be volatile or for the write to be volatile.
-
Groo about 13 years@Eric: But if the first read is not volatile, the only thing that can happen is that several threads could end up wait at
lock
, and then return as soon as they are granted the lock. I was referring to the general problem of double-checked locking due to compiler reordering of statements, which apparently isn't a problem in .NET (as described at the bottom of this article). -
Eric Lippert about 13 years@Groo: Why is that the "only" thing that could happen given that we have not been shown either the implementation of _DoSomething or the implementation of the caller? What prevents the caller from accessing memory mutated by DoSomething? And what prevents those non-volatile memory accesses from being moved around in time to before the first read of "done" on a thread that never enters the lock (because "done" is true)? I don't think any of those things are prevented. However, I am not an expert on this subject; if you'd like to show me where the error is in my analysis I'd appreciate it.
-
Thomas Levesque about 13 yearsEric, how do you suggest to perform the volatile read then? Using Thread.VolatileRead?
-
Eric Lippert about 13 years@Thomas: I suggest never using double checked locking unless your name is Joe Duffy. If you do want to live dangerously and invent your own versions of double-checked locking then you probably don't want to use Thread.VolatileRead since it can have bad performance characteristics -- in some implementations I believe it creates a full memory fence, though I might be misremembering. You probably want to simply make "done" volatile, even though that makes some of its reads and writes unnecessarily volatile.
-
Eric Lippert about 13 years@Groo: re, that MSDN article. That is a completely different pattern. I know it looks the same, but from the processor's perspective that is completely different code. The pattern on the MSDN page only reads from one storage location and checks it for null. The code here could read from two storage locations which might be on completely different memory pages. Being four bytes apart is as good as being a billion bytes apart if those four bytes straddle a page boundary. The MSDN version is safe; clearly it's impossible to get inconsistent read order of a single location.
-
Groo about 13 years@Eric, @Thomas: You are right Eric, I was talking nonsense. I didn't take enough time to think about the code posted. I had the singleton construction pattern in mind, with a null check of the field, and the newly created instance assigned inside the lock. Actually, the way this code is written right now is wrong, since
done
is set totrue
before_DoSomething
is even executed, so there may be a number of threads which would presume that work was done. -
Eric Lippert about 13 years@Groo: Right, that's the larger problem; I posted a comment a few minutes ago to the OP asking why they want to return early if the work is still being done. That seems like a misfeature to me.
-
Groo about 13 yearsI believe the proper way is not only to put the
done = true
statement at the end, but to add aSystem.Threading.Thread.MemoryBarrier();
statement after the call to_DoSomething
. Otherwise, if the method gets inlined and statements reordered,done
might be set totrue
too early. -
Daniel about 13 yearsYour volatile and memory sharing explanation helped clear some other bugs I had, but the other threads don't need to use any information generated by ReallyDoSomething.
-
PVitt over 10 years@EricLippert Thanks. I just adapted the answer.
-
Eric Lippert over 10 yearsThe edit makes the code incorrect. Suppose a thread abort exception is thrown after the try is entered but before the lock is taken. The finally block will run and release a lock that was never taken.