[Mono-devel-list] Patch for Thread.GetNamedDataSlot

Sébastien Robitaille sebastien.robitaille at croesus.com
Thu Jul 15 11:08:13 EDT 2004

Hi Jonathan,

thanks for the review.
For your information, I used lock(typeof(Thread)) in GetNamedDataSlot because there is the same kind of locking in InitDataStoreHash, AllocateNamedDataSlot and FreeNamedDataSlot. I agree with you that "lock(typeof(Thread)) should not be used .. That means that these methods must also be modified...

Before I submit an updated patch with these methods, I need advice on this:
I can use a different object for the lock as suggested. But since there is already a static Hashtable object available (which all these methods are using), I could use it instead (to avoid allocating a new object). But in this scenario, the Hashtable object would have to be allocated directly at the declaration instead of using the InitDataStoreHash() method "on demand"...

private static Hashtable datastorehash = new Hashtable();

I think this scenario is more clean, but I don't know the cost of always allocating a Hashtable object vs always allocating a System.Object.

Another point: In these methods, the HashTable object is created Synchronized. I think that it is superfluous since every access to that hashtable is locked anyway...

All comments/suggestions are welcome.


-----Original Message-----
From: mono-devel-list-admin at lists.ximian.com
[mailto:mono-devel-list-admin at lists.ximian.com]On Behalf Of Jonathan
Sent: Thursday, July 15, 2004 7:08 AM
To: Sébastien Robitaille
Cc: Mono Development List; dick at ximian.com
Subject: Re: [Mono-devel-list] Patch for Thread.GetNamedDataSlot

On Wed, 2004-07-14 at 21:14, Sébastien Robitaille wrote:
> The problem: Thread.GetNamedDataSlot is not thread-safe. There is an
> exception thrown if multiple threads are calling this method at
> the same time.


+                       lock(typeof(Thread))
+                       {
+                               if (datastorehash == null)


You shouldn't lock the Thread class.  It is error prone and can lead to
deadlock.  You should instead lock a private object:

	private static object _lock = new object ();

	// ...
		lock (_lock) {
			// ...

See also:

 - Jon

Mono-devel-list mailing list
Mono-devel-list at lists.ximian.com

More information about the Mono-devel-list mailing list