MySQL Bugs: #40231: GetDefaultCollation Thread sync issue

Bug #40231	GetDefaultCollation Thread sync issue
Submitted:	22 Oct 2008 7:32	Modified:	13 Nov 2008 9:16
Reporter:	Christos Pavlides	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Connector / NET	Severity:	S3 (Non-critical)
Version:	5.3.0.0	OS:	Windows
Assigned to:	Reggie Burnett	CPU Architecture:	Any
Tags:	GetDefaultCollation

Description:
GetDefaultCollation and GetMaxLength call the db to get a set of parameters and cache them in two static dictionaries in function InitCollections. These functions are not thread safe since if many threads try to call a procedure for the first time they will all try to insert the same keys in these collections and a lot of duplicate key exceptions will be thrown.

How to repeat:
It is relatively clear from the code that this can happen. Unfortunately this happened a long time ago and I do not have the time currently to create a test case. I have a fix in my code base which works fine.
But to repeat the issue you can open up a number of connections to the server using multiple threads (50-100 threads). Then try to run a few procedures. Running a procedure requires the client to find out the params of the procedure which in turn makes a call to GetProcedureParameters from the ISSchemaProvider which calls the GetDefaultCollation.
Theoretically this path should be taken only once, on the first call the procedure and then the procedure properties are cached. But if you open a number of connections and call the same procedure over and over again, there is a high propability that two threads will try to call the GetDefaultCollation at the same time.
Since the method is static the collections are static therefore we have to lock the collections at some point. Since this is called only once during the lifetime of the host application, it is not a bottleneck to lock the entire collection or the entire class for the duration of the GetDefaultCollation.( This is actually what I did to fix it) 

Suggested fix:
I defined a defaultCollationslock static object and I lock it as soon as the InitCollections is called. 

internal static void InitCollections(MySqlConnection connection)
        {
            lock (defaultCollationslock)
            {
                if (defaultCollations != null)
                    return;
                defaultCollations = new Dictionary<string, string>();
                maxLengths = new Dictionary<string, int>();

                MySqlCommand cmd = new MySqlCommand("SHOW CHARSET", connection);
                using (MySqlDataReader reader = cmd.ExecuteReader())
                {
                    while (reader.Read())
                    {
                        defaultCollations.Add(reader.GetString(0), reader.GetString(2));
                        maxLengths.Add(reader.GetString(0), Convert.ToInt32(reader.GetValue(3)));
                    }
                }
            }
        }

This is also present in 5.2.3.
The severity should be upped as this keeps this version of the connector from being usable in applications with a high degree of concurrency.

I have not seens this problem in prior versions but it has started showing up after upgrading the connector to 5.2.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/58225

fixed in 5.2.4+

An entry was added to the 5.2.4 changelog:

GetDefaultCollation and GetMaxLength were not thread safe. These functions called the database to get a set of parameters and cached them in two static dictionaries in the function InitCollections. However, if many threads called them they would try to insert the same keys in the collections resulting in duplicate key exceptions.

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/58505

Thanks for the fix. but I cannot see the changes in the trunk. Do you know when the changes will be merged?

merged to trunk this morning

Hi Reggie, the last commit has a bug. Basically you are trying to lock the defaultCollations collection but it is null, therefore an exception is thrown. To fix this I used a similar method as the one I described initially. Basically  I created a static object and I initialized it. Then I use that object to lock. This way you are always locking the same object which is guaranteed to be not null.

internal static string GetDefaultCollation(string charset, MySqlConnection connection)
        {
            lock (lockDefaultCollations)
            {
                if (defaultCollations == null)
                    InitCollections(connection);
            }
            if (!defaultCollations.ContainsKey(charset))
                return null;
            return defaultCollations[charset];
        }

        internal static int GetMaxLength(string charset, MySqlConnection connection)
        {
            // we lock on defaultCollations here too so GetDefaultCollation
            // is on the same lock as us.
            lock (lockDefaultCollations)
            {
                if (maxLengths == null)
                    InitCollections(connection);
            }

            if (!maxLengths.ContainsKey(charset))
                return 1;
            return maxLengths[charset];
        }

Christos

Yes, I found that in a round of testing after the commit and committed a fix but that fix has not yet been merged to trunk.