Bug #40231 GetDefaultCollation Thread sync issue
Submitted: 22 Oct 2008 9:32 Modified: 13 Nov 2008 10:16
Reporter: Christos Pavlides
Status: Closed
Category:Connector/Net Severity:S3 (Non-critical)
Version:5.3.0.0 OS:Microsoft Windows
Assigned to: Reggie Burnett Target Version:
Tags: GetDefaultCollation

[22 Oct 2008 9:32] Christos Pavlides
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)));
                    }
                }
            }
        }
[6 Nov 2008 10:08] Sean Kinsey
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.
[7 Nov 2008 20:17] 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/58225
[7 Nov 2008 20:18] Reggie Burnett
fixed in 5.2.4+
[10 Nov 2008 15:50] Tony Bedford
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.
[11 Nov 2008 23:03] 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/58505
[12 Nov 2008 8:03] Christos Pavlides
Thanks for the fix. but I cannot see the changes in the trunk. Do you know when the
changes will be merged?
[12 Nov 2008 19:55] Reggie Burnett
merged to trunk this morning
[13 Nov 2008 10:16] Christos Pavlides
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];
        }
[13 Nov 2008 13:50] Reggie Burnett
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.