Bug #23905 Stored procedure usages is not thread safe
Submitted: 2 Nov 2006 19:45 Modified: 14 Dec 2006 14:52
Reporter: Karl Seguin Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / NET Severity:S2 (Serious)
Version:5.0.1.0 OS:Windows (Windows)
Assigned to: CPU Architecture:Any
Tags: .net, multithreading, stored procedure

[2 Nov 2006 19:45] Karl Seguin
Description:
ProcedureCache.GetProcedure isn't thread safe. If two threads enter the function for the same stored procedure, an ArgumentException is very likely to be raised as no locking is taking place.

DataSet set = (DataSet) procHash[num1];
if (set == null)
{
   set = AddNew(conn, spName);
   conn.PerfMonitor.AddHardProcedureQuery();
   //...
   return set;
}

Since no lock is being acquired, 2 threads will get null from procHash, both will enter AddNew, and both will try to add data into the same hash using the same key. An ArgumentException will be thrown with a message like:

 Key in dictionary: '-1231232'  Key being added: '-1231232'

How to repeat:
Threading issues are hard to reproduce, but AddNew is slow enough (relatively speaking) that the simplest load test will cause the race condition.

Suggested fix:
As a workaround, developers can catch and swallow ArgumentException when executing a sproc, or implement their own locking.

Developers could also load the sproc data into cache before starting multiple threads, although this won't always be possible.

The real fix is to lock around the hashtable insert.

instead of:

int num = spName.GetHashCode();
procHash.Add(num, set);

the code should be

int num = spName.GetHashCode();
if (!procHash.ContainsKey(num))
{
   lock(_someLockingObject)
   {
       if (!procHash.ContainsKey(num))
       {
           procHash.Add(num, set);
       }
   }
}
[2 Nov 2006 20:42] Karl Seguin
Disabling connection pooling (;pooling=false in the connection string) is also a good work-around, since a new procedureCache is created for each connection.

So this is specifically a problem with multithreaded applications using connection pooling.
[24 Nov 2006 10:12] Tonci Grgin
Hi Karl and thanks for your report.
Verified as described by reporter with latest sources from Trunk:
		private DataSet AddNew(MySqlConnection connection, string spName)
		{
			DataSet procData = GetProcData(connection, spName);
			if (maxSize > 0)
			{
				if (procHash.Keys.Count == maxSize)
					TrimHash();
				int hash = spName.GetHashCode();
				procHash.Add(hash, procData);
				hashQueue.Enqueue(hash);
			}
			return procData;
		}
[4 Dec 2006 7:22] Nalle Jacobsson
This bug is not only in 5.0.1.0, it's also present in 1.0.8 RC. I'm getting this error all the time since I upgraded from 1.0.7. Until it's fixed - how do I best deal with it? Just catch the error and ignore it?
[10 Dec 2006 12:58] Juan Pablo Garcia
Thanks, i have the same problem, and i have been able to solve it momentarily with lock(object). In my case to reproduce this error, i use the Microsoft Web Application Stress Tool, configuring it with two or more threads.

Now... my problem is solved, but i'm not 100% satisfied, because with the lock(object) i lost the capabillity of use multithreading and the performance it's like using POOLING=FALSE.

I've tried to use connector 1.0.8 and 5.0 and with various versions of mysql incluing the last 5.1.12 beta. The server machine is a Windows 2003. And using Framework 2.0

My error log look's like:

System.ArgumentException: Item has already been added. Key in dictionary: '-87286485'  Key being added: '-87286485'
   at System.Collections.Hashtable.Insert(Object key, Object nvalue, Boolean add)
   at System.Collections.Hashtable.Add(Object key, Object value)
   at MySql.Data.MySqlClient.ProcedureCache.AddNew(MySqlConnection connection, String spName) in C:\Inetpub\wwwroot\xxx\MySQL_Connector_5.0.2\ProcedureCache.cs:line 113

Any suggestions?
[13 Dec 2006 11:28] Markus Wolters
Just a little marker: Change in constructor ProcedureCache() this line:

procHash = new Hashtable(maxSize);

to this line:

procHash = Hashtable.Synchronized(new Hashtable(maxSize));

This belongs to 1.0.8RC, but probably the code in 5.0.X is similar.

Regards,
Markus
[13 Dec 2006 17:30] Reggie Burnett
Markus

That won't fix it.  Doing what you suggest makes the collection thread safe but it still will not prevent two threads from querying for a proc at the same time, both discovering that the proc has not been cached yet, and both trying to query and cache the proc resulting in the hash clash reported.
[13 Dec 2006 17:52] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/16901
[13 Dec 2006 17:56] Reggie Burnett
Fixed in 1.0.9 and 5.0.3
[13 Dec 2006 17:56] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/16902
[14 Dec 2006 14:52] MC Brown
A note has been added to the 1.0.9 and 5.0.3 changelogs.