These 3 types of lock are apparently bad. What other type of locking is bad? Are there Stylecop / FxCop rules that would catch this? If not, then would you please help me with a custom rule implementation? They code for all of them must be similar, right?
Thank you.
Basically, you should not lock on any external object unless this is specifically a locking object (such as the SyncRoot property on the non-generic ICollection was designed for). Doing so carries the risk of other "users" of the reference also locking on it, leading to unwanted locking or even deadlocks.
Obvisously, this and typeof() are by definition external objects. Strings are immutable and string literals are all interned, so that the same reference can be in unse at different places even if you directly assigned it in your object.
I don't know of a StyleCop rule for those, but I don't have a good overview of what is available for StyleCop or FxCop, so there could very well be something in the wild to check for those cases. I'd check for synching only on private members which are not strings and not directly returned in any property or method.
The samples of John Robbins' Debugging Microsoft .NET Applications book contain sources for such FxCop rules (DoNotLockOnPublicFields, DoNotLockOnThisOrMe, DoNotLockOnTypes, etc.). It looks like they were originally made for FxCop 1.35, whereas the version in VS 2008 and the latest standalone version is 1.36 (not to speak of VS2010). So they might need some tweaking, YMMV.