MySQL Bugs: #23905: Stored procedure usages is not thread safe

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

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);
       }
   }
}

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.

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;
		}

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?

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?

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

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.

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

Fixed in 1.0.9 and 5.0.3

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

A note has been added to the 1.0.9 and 5.0.3 changelogs.