N
Nashat Wanly
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnaskdr/html/askgui06032003.asp
Don't Lock Type Objects!
Why Lock(typeof(ClassName)) or SyncLock GetType(ClassName) Is Bad
Rico Mariani, performance architect for the Microsoft® .NET runtime
and longtime Microsoft developer, mentioned to Dr. GUI in an e-mail
conversation recently that a fairly common practice (and one that's,
unfortunately, described in some of our documentation right now,
although we'll be fixing that) is actually quite problematic. He asked
if Dr. GUI could help get the word out that programmers shouldn't do
this. The good doctor was delighted to oblige.
What is this fairly common practice? Well, it's getting a lock on a
type object. In C#, it's lock(typeof(ClassName)) where ClassName is
the name of some class; in Microsoft® Visual Basic® .NET, it's
SyncLock GetType(ClassName).
A bit of background: The lock/SyncLock statement is used in
multithreaded programming to create critical sections, or brief
sections of your code where only one thread can execute at a time.
(You might need this if you had to update more than one field in your
object simultaneously—you'd want to make sure that another thread
didn't try to update the object at the same time!) This statement
locks the unique monitor object associated with the object you
specify, waiting if another thread has the monitor already. Once it
locks the monitor, no other thread can lock it until your thread
releases the lock, which happens automatically at the end of enclosed
block. A common usage is to lock the this/Me reference so that only
your thread can modify the object you're using—or better yet, to lock
the specific object you're about to modify. Locking the smallest
possible object is good because it helps to avoid needless waiting.
GetType and typeof return a reference to the type object for that
type. The type object, of type System.Type, contains methods that
allow you to reflect on the type, meaning you can find its fields and
methods, and even access fields and call methods. You can even create
an instance of the object once you have a reference to the type object
(and you can get a reference to a type object by name if you use the
Type.GetType shared/static method).
So the type object is pretty handy. But some programmers have taken to
using it as a "cheap" way of getting a proxy for a static/Shared
object you can put a lock on. (And, unfortunately, we document this in
both the C# and Visual Basic .NET documentation, implying that it's a
recommended practice.) In this case, the docs are wrong (and we'll be
correcting them). This practice is not acceptable, let alone
recommended.
Here's why: Since there's one type object for all instances of a
class, it would appear that locking it would provide a lock equivalent
to locking a static object contained in your class. You would lock all
instances of the class, wait until other threads were done accessing
any part of any instance, then lock access so you could access static
members safely and without another thread interfering.
And it does work, at least most of the time. But there are problems
with it: First, getting the type object is actually a fairly slow
process (although most programmers would guess that it's extremely
fast); second, other threads in ANY CLASS and even different programs
running in the same application domain have access to the type object,
so it's possible that they'll lock the type object on you, blocking
your execution entirely and causing you to hang.
The basic problem here is that you don't own the type object, and you
don't know who else could access it. In general, it's a very bad idea
to rely on locking an object you didn't create and don't know who else
might be accessing. Doing so invites deadlock. The safest way is to
only lock private objects.
But wait; it's even worse than all that. As it turns out, type objects
are sometimes shared across application domains (but not across
processes) in current versions of the .NET runtime. (This is generally
okay since they're immutable.) That means that it's possible for
ANOTHER APPLICATION running even in a different application domain
(but in the same process) to deadlock your application by getting a
lock on a type object you want to lock and never releasing it. And it
would be easy to get access to that type object because the object has
a name—the fully qualified name of the type! Remember that
lock/SyncLock blocks (that's a polite word for hangs) until it can
obtain a lock. It's obviously really quite bad to rely on a lock that
another program or component can lock and cause you to deadlock.
Even if the type objects were unique to your application domain, this
would still be a bad practice because any code could get access to the
type object for a public type and cause a deadlock. This is especially
a problem when you use components in your application that you didn't
write. (Even lock(this)/SyncLock Me can have this problem, since
someone else can lock you. Although if that happens, it's likely to be
easier to find than the deadlock caused by locking a type object,
since your object isn't globally available across application
domains.)
So what should you do instead? Well, it's pretty easy: just declare
and create an object to use as a lock, then use it, not the type
object, to do your locking. Usually, to duplicate the intended
semantics of the bad code, you'll want this object to be
static/Shared—and it really, of course, should be private! In general,
you could change the following bad code:
// C#
lock(typeof(Foo)) { // BAD CODE! NO! NO! NO!
// statements;
}
' VB .NET
SyncLock GetType(MyClass) ' BAD CODE! NO! NO! NO!
' statements
End SyncLock
…into the following good code:
// C#
lock(somePrivateStaticObject) { // Good code!
// statements;
}
' VB .NET
SyncLock GetType(somePrivateStaticObject) ' Good code!
' statements
End SyncLock
Of course, you'll have to either have a private static object to lock
already (if you're using the lock to modify static objects, you may in
fact already have one!) or you'll have to create one. (Make it private
to keep other classes from locking your object.) Do NOT attempt to
lock a field that's not a reference (object) type, such as
int/Integer. You'll get a compiler error. If you don't have a private
static object to lock, you may need to create a dummy object:
// C#
Class MyClass {
private static Object somePrivateStaticObject = new Object();
// methods of class go here--can lock somePrivateStaticObject
}
' VB .NET
Class MyClass
Private Shared somePrivateStaticObject As New Object
' methods of class go here--can lock somePrivateStaticObject
End Class
You'll want to analyze each case separately to make sure you've got it
right, but in general, the above technique should work.
Note a couple of things: First, no code outside of your class can lock
MyClass.somePrivateStaticObject, so you've eliminated many
opportunities for deadlock. And deadlocks are among the nastiest bugs
to find, so eliminating opportunities for them is a very good thing.
Second, you know that there's exactly one copy of
MyClass.somePrivateStaticObject for your application and exactly one
for each other application on the system that is running, so there's
no interplay across applications in the same application domain. Dr.
GUI hopes you can see why this code is much more reliable and robust
than the bad code.
To summarize, don't lock type objects. You don't know where they've
been. Doing so is slow and can expose you to possible deadlock
situations. It's a bad programming practice. Instead, lock a static
object in your object.
Don't Lock Type Objects!
Why Lock(typeof(ClassName)) or SyncLock GetType(ClassName) Is Bad
Rico Mariani, performance architect for the Microsoft® .NET runtime
and longtime Microsoft developer, mentioned to Dr. GUI in an e-mail
conversation recently that a fairly common practice (and one that's,
unfortunately, described in some of our documentation right now,
although we'll be fixing that) is actually quite problematic. He asked
if Dr. GUI could help get the word out that programmers shouldn't do
this. The good doctor was delighted to oblige.
What is this fairly common practice? Well, it's getting a lock on a
type object. In C#, it's lock(typeof(ClassName)) where ClassName is
the name of some class; in Microsoft® Visual Basic® .NET, it's
SyncLock GetType(ClassName).
A bit of background: The lock/SyncLock statement is used in
multithreaded programming to create critical sections, or brief
sections of your code where only one thread can execute at a time.
(You might need this if you had to update more than one field in your
object simultaneously—you'd want to make sure that another thread
didn't try to update the object at the same time!) This statement
locks the unique monitor object associated with the object you
specify, waiting if another thread has the monitor already. Once it
locks the monitor, no other thread can lock it until your thread
releases the lock, which happens automatically at the end of enclosed
block. A common usage is to lock the this/Me reference so that only
your thread can modify the object you're using—or better yet, to lock
the specific object you're about to modify. Locking the smallest
possible object is good because it helps to avoid needless waiting.
GetType and typeof return a reference to the type object for that
type. The type object, of type System.Type, contains methods that
allow you to reflect on the type, meaning you can find its fields and
methods, and even access fields and call methods. You can even create
an instance of the object once you have a reference to the type object
(and you can get a reference to a type object by name if you use the
Type.GetType shared/static method).
So the type object is pretty handy. But some programmers have taken to
using it as a "cheap" way of getting a proxy for a static/Shared
object you can put a lock on. (And, unfortunately, we document this in
both the C# and Visual Basic .NET documentation, implying that it's a
recommended practice.) In this case, the docs are wrong (and we'll be
correcting them). This practice is not acceptable, let alone
recommended.
Here's why: Since there's one type object for all instances of a
class, it would appear that locking it would provide a lock equivalent
to locking a static object contained in your class. You would lock all
instances of the class, wait until other threads were done accessing
any part of any instance, then lock access so you could access static
members safely and without another thread interfering.
And it does work, at least most of the time. But there are problems
with it: First, getting the type object is actually a fairly slow
process (although most programmers would guess that it's extremely
fast); second, other threads in ANY CLASS and even different programs
running in the same application domain have access to the type object,
so it's possible that they'll lock the type object on you, blocking
your execution entirely and causing you to hang.
The basic problem here is that you don't own the type object, and you
don't know who else could access it. In general, it's a very bad idea
to rely on locking an object you didn't create and don't know who else
might be accessing. Doing so invites deadlock. The safest way is to
only lock private objects.
But wait; it's even worse than all that. As it turns out, type objects
are sometimes shared across application domains (but not across
processes) in current versions of the .NET runtime. (This is generally
okay since they're immutable.) That means that it's possible for
ANOTHER APPLICATION running even in a different application domain
(but in the same process) to deadlock your application by getting a
lock on a type object you want to lock and never releasing it. And it
would be easy to get access to that type object because the object has
a name—the fully qualified name of the type! Remember that
lock/SyncLock blocks (that's a polite word for hangs) until it can
obtain a lock. It's obviously really quite bad to rely on a lock that
another program or component can lock and cause you to deadlock.
Even if the type objects were unique to your application domain, this
would still be a bad practice because any code could get access to the
type object for a public type and cause a deadlock. This is especially
a problem when you use components in your application that you didn't
write. (Even lock(this)/SyncLock Me can have this problem, since
someone else can lock you. Although if that happens, it's likely to be
easier to find than the deadlock caused by locking a type object,
since your object isn't globally available across application
domains.)
So what should you do instead? Well, it's pretty easy: just declare
and create an object to use as a lock, then use it, not the type
object, to do your locking. Usually, to duplicate the intended
semantics of the bad code, you'll want this object to be
static/Shared—and it really, of course, should be private! In general,
you could change the following bad code:
// C#
lock(typeof(Foo)) { // BAD CODE! NO! NO! NO!
// statements;
}
' VB .NET
SyncLock GetType(MyClass) ' BAD CODE! NO! NO! NO!
' statements
End SyncLock
…into the following good code:
// C#
lock(somePrivateStaticObject) { // Good code!
// statements;
}
' VB .NET
SyncLock GetType(somePrivateStaticObject) ' Good code!
' statements
End SyncLock
Of course, you'll have to either have a private static object to lock
already (if you're using the lock to modify static objects, you may in
fact already have one!) or you'll have to create one. (Make it private
to keep other classes from locking your object.) Do NOT attempt to
lock a field that's not a reference (object) type, such as
int/Integer. You'll get a compiler error. If you don't have a private
static object to lock, you may need to create a dummy object:
// C#
Class MyClass {
private static Object somePrivateStaticObject = new Object();
// methods of class go here--can lock somePrivateStaticObject
}
' VB .NET
Class MyClass
Private Shared somePrivateStaticObject As New Object
' methods of class go here--can lock somePrivateStaticObject
End Class
You'll want to analyze each case separately to make sure you've got it
right, but in general, the above technique should work.
Note a couple of things: First, no code outside of your class can lock
MyClass.somePrivateStaticObject, so you've eliminated many
opportunities for deadlock. And deadlocks are among the nastiest bugs
to find, so eliminating opportunities for them is a very good thing.
Second, you know that there's exactly one copy of
MyClass.somePrivateStaticObject for your application and exactly one
for each other application on the system that is running, so there's
no interplay across applications in the same application domain. Dr.
GUI hopes you can see why this code is much more reliable and robust
than the bad code.
To summarize, don't lock type objects. You don't know where they've
been. Doing so is slow and can expose you to possible deadlock
situations. It's a bad programming practice. Instead, lock a static
object in your object.