I have been looking into the claim that the keyword lock is not safe
when exceptions are possible. That lead me to try the following code,
which I think has uncovered a serious error in the .NET 2.0 framework.
Note that this runs better, but not perfectly, on .NET 1.1.
Note: The numbers are line numbers needed to match the callstack of the
// Class WorkItem ...
34 public void DoItSafeLock()
43 Monitor.Exit(this); // <-- SynchronizationLockException!
Here's what I get when I run this code on a different thread than the
one that created the object holding this method (WorkItem class):
System.Threading.SynchronizationLockException: Object synchronization
method was called from an unsynchronized block of code.
at WorkItems.WorkItem.DoItSafeLock() in
at WorkItems.Worker.ExecuteWorkItem() in
Notice that the error is that on line 43, the object does not hold a
lock on itself (this). How is this possible given that it made it
through the Monitor.Enter(this) call on line 38?!?
Does any one know what's going on here? How can the monitor be entered
and yet no lock is acquired?
I have a sample application demonstrating the error if any one wants to
see the code, just email me and I'll zip up the solution and send it
Thanks in advance,
That is not the problem. It is a best practice to not do this, but it
should not fail .NET's Monitor class. Thanks for the advice though.
ps - Once classes "other object" is another classes this.
It's kind of big, across 4 files. Can I email it to you?
Ok, just a second. I'll send it up for you guys.
You can download the solution here:
Let me know what you think,
In case you didn't see the reponse to Nicholas...
I will run this at home later, since I don't have vs2005 on this
Sorry if my first answer was on the glib side.
Thanks. Let me know what you think. I could be missing something, but I
think the code is correct, and yet it's kickin' exceptions out left and
If you want to see what it's output is like without VS.NET 2005, you
could look at the SampleOutput.txt file including in one of the
When your thread gets aborted when Monitor.Enter(this) (in DoItSafeLock())
is waiting to aquire the lock, the wait will be abandoned and the finally
clause ( so Monitor.Exit( this );) executed, resulting in a
ASP.NET only aborts threads as a result of Apllication Domain unloads, that
means that the domain and it's loaded code and objects will be removed
Clients of your library code should not abort threads they don't own, this
is true for unmanaged code as well as for managed code.
Note that V2 of the framework contains some reliability features to protect
you from issues resulting from asynchronous thread abort's (proper clean-up
of unmanaged resources and locks), but again this doesn't mean it's safe to
introduce asynchronous thread aborts while continuing to execute code in
Please read my reply in the clr NG where you posted the same question.
That is not true. Response.Redirect(url) or Response.Redirect(url,
true) will also throw thread abort exceptions.
One more thing:
> Clients of your library code should not abort threads they don't own, this
> is true for unmanaged code as well as for managed code.
True, but consider this.
1. Client creates a thread
2. Clent creates an object from my library and interacts with it on
3. My objects uses lock properly.
4. Client aborts *their* thread.
5. My object is dead-locked.
I said you can only safely call Thread.Abort in the code executing on that
thread (synchronously). A thread started by a client, does not mean the
client owns the thread, it's the thread procedure that owns the thread it's
That means calling Thread.CurrentThread.Abort() is safe, anything else is
unsafe and should only result from a AD unload.
Sure, but, Response.Redirect(url) and Response.Redirect(url), do throw a
ThreadAbortException they don't call Thread.Abort().
That means that the CURRENT thread (the one that throws) will terminate as
ThreadAbortException cannot be catch'd, this is totally different from
calling SomeOtherThread.Abort() on another thread, which injects a
ThreadAbortException in "SomeOtherThread" thread.
If possible, don't give clients of your code the opportunity to
suspend/start/stop/abort (etc.) your threads, or lock your classes'
objects. What a client doesn't know can't hurt them. If they call
YourObject.Foo(), and that starts a new thread running the Bar()
method, so be it. The caller doesn't need to know this.
Hope this helps, even if it's not 100% correct in every detail.
// do something
Done the original way, surely you don't know (in the finally block where you
release the lock) that you have actually taken the lock; with the above code
you know that if you made it to the try block you *have* the lock...
Apologies if this is my naivety showing through and the above is in fact
"Marc Gravell" <mgra...@rm.com> wrote in message
Actually, in .NET 1.1 Monitor.Exit is completely broken - the docs
claim it throws SynchronizationLockException if the current thread
doesn't own the lock, but in practice it doesn't.
What you're seeing is a consequence of Monitor.Exit doing the *right*
thing in 2.0, combined with the bad practice of aborting (or
interrupting - that can have the same effect) another thread.
See http://www.pobox.com/~skeet/csharp/threads/abort.shtml for more
I think you've convinced me to stop worrying.
My main concert is just that someone would consume my object in their
thread, then abort their thread and effectively break my threading
logic. But after talking with you guys I've seen basically you're
screwed either way entery/try or try/enter.
So I'll just hope no one does that and they get what they get if they
>What you're seeing is a consequence of Monitor.Exit doing the *right*
>thing in 2.0, combined with the bad practice of aborting (or
>interrupting - that can have the same effect) another thread.
Now that I've thought about this for awhile, I see that tou're right.
Thanks for the info!
I am familiar with design patterns, but I don't know of anything I
could do to avoid someone creating their own thread, using my objects
(which may not even use a thread directly), aborting their thread, and
breaking my usage of lock().
See my response to Willy for a little more on that:
Thanks for the help!
Yeah, that part isn't really the issue (this vs lockObj). As Willy has
convinced me, you're basically helpless to keep things working right
once you go down the path to Thread.Abort(). try/enter/... or
enter/try/... either way results in unbalanced enter/exit calls.