[Mono-devel-list] [PATCH] Warn on lock (string) or lock (typeof (Foo))

[Ben Maurer]

On Fri, 2005-05-27 at 18:33 -0400, Jonathan Pryor wrote:
> On Fri, 2005-05-27 at 15:43 -0400, Ben Maurer wrote:
> > Hello,
> > 
> > The attached patch makes mcs emit an added-value warning: it warns when
> > you lock on a string or System.Type.
> 
> Good idea overall, though I'd suggest a slight change warn whenever you
> lock on a public member.  This would catch both System.Type and string
> (both are public members), and it's generally not safe to lock on public
> members for the same reasons locking on string or System.Type is unsafe
> (increased possibility for deadlock being the primary issue).

Locking on, eg, SyncRoot (public property) of a private hashtable is
correct. Also, locking on a public member of an internal class could
possibly be correct.

Really, a tool that does some sort of escapes analysis is needed. For
example this code

internal class X {
	ArrayList ar;
	IUnknownObject foo;

	void Blah ()
	{
		lock (ar.SyncRoot) {
			foo.Method (ar);
		}
	}
}

Would need to be flagged. Even though ar is a "private" object, it gets
exposed to the outside world via IUnknownObject.

Also, we need to do a poison check of sorts: if the function gets an
object from an unknown place, its a bad idea to lock on it.

In some ways, we could even consider warning on locking anything other
than a private object. However, thats a bit extreme. 

-- Ben