Bug #31996 Connection pooling - MySqlConnection object release
Submitted: 31 Oct 2007 16:40 Modified: 16 Dec 2009 13:20
Reporter: Umberto Ballestrazzi Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / NET Severity:S3 (Non-critical)
Version:5.0 latest sources OS:Any
Assigned to: Vladislav Vaintroub CPU Architecture:Any

[31 Oct 2007 16:40] Umberto Ballestrazzi
Description:
The connection is not released when is not referenced.
If you run the attached program you see that at the end there are 30 connections opened to MySql.

A workaround is to be sure that every connection is closed (even on exceptions and so on).

Anyway the behaviour should be that the connection are released by garbage collector.

How to repeat:
      MySqlConnection conn1;

      for (int i = 0; i < 30; i++)
      {
        conn1 = new MySqlConnection("host=frogger;user=root;password=dacambiare;");
        conn1.Open();

        conn1 = null;

        GC.Collect();

        //conn1.Close();
      }

      Console.WriteLine("Press enter to exit");
      Console.ReadLine();

Suggested fix:
There are 2 problems.
The finalizer of the MySqlConnection (probably is implemented by Component but I do not remember) is not called because the MySqlConnection object is referenced by the Driver (that is in the connection pool so is referenced as well). It is necessary to remove the reference to the connection in the driver. Doing it the Dispose of MySqlConnection is called by the garbage collector thread.

The second problem is that the dispose implementation in connection is not right. The Close statement should be called always (if the connection is still open that means if the object is not disposed).
[1 Nov 2007 17:29] Reggie Burnett
This is not a bug since you have not disabled pooling.  Even though you set the connection object to null and perhaps the connection object is collected, the internal driver object remains open against the server so that future calls to Open() will use a pooled connection and be very fast.  If you want the physical connection to close then you need to disable pooling by setting pooling=false on your connection string.

BTW, what version of the connector did you report this on?
[3 Nov 2007 10:24] Umberto Ballestrazzi
Hi Reggie,
the issue is that in this case the connection pooling does not work.
In the example I dereferenced all the 30 (29) connection (so I will never be able to reuse them) but the connection pooling will never reuse them as well.

The problem is related to the finalizer and to the Dispose method (but I already posted it). The finalizer is never called so the connection pooling does not understand that the connections could be reused (ergo it does not work).
[3 Nov 2007 10:30] Umberto Ballestrazzi
Ops...
I forgot the version...
The newest source code date is 31/08, probably 5.0.8.1
[3 Nov 2007 10:41] Umberto Ballestrazzi
Hi Reggie,
reading source code of the latest release (5.1) there is exactly the same problem.
The driver is referencing the MySqlConnection so the garbage collector will never call the Finalize so the connection pooling will never know that the connection can be reused.
[31 Dec 2007 13:24] Tonci Grgin
Hi Umberto. I believe you are trying to use pooling mechanism in wrong way thus misunderstanding between you and Reggie. Pool management classes have been added to c/NET 5.2 so you may try it and see if it fits your needs:
Version 5.2
. Added ClearPool and ClearAllPools features
[31 Dec 2007 14:12] Umberto Ballestrazzi
Hi Tonci,
the problem is what I described (and for me not solved).
By the way, there are some other cases that arises it so after some days the server have to be restarted (otherwise the connection pooling must be disabled).
I could not wait anymore for a fix so 1 month ago I fixed it (hoping in an official fix).
It's a structural change but it takes less than an hour.
If you need the updated sources I can download latest, apply the fix and send it to you. 
Thanks again for your support,
Umberto
[31 Dec 2007 14:25] Tonci Grgin
Umberto, please feel free to attach your patch / explanation. I'll make sure it gets reviewed.

Have a nice holidays and thanks for your interest in MySQL.
[31 Dec 2007 18:05] Umberto Ballestrazzi
Tonci,
I just have looked the patch.
It's several files.
I can do it but if you follow my suggestions (in this bug) it is simple to do it by yourself.
Umberto
[28 Jan 2008 10:40] Andrew A
A simple test case for this should be that after setting all connections = null (as per above) without closing, and then attempting to open one more connection (pool size + 1), a pool timeout exception should be thrown.
[5 Feb 2008 9:56] Tonci Grgin
Guys, thanks very much for spotting this. It certainly appears there's a misbehavior of connection pool... Thanks again and sorry for the delay in processing. Now we'll see what Reggie has to say.

Verified as described with MySQL server 5.0.54BK on WinXP Pro SP2 localhost. I used .NET fw 2 SP1 with c/NET 5.0 latest sources.

String connectString = "server=localhost;port=3306;database=test;user id=root;password=*****;Pooling=True;Min Pool Size=1;Max Pool Size=30;connect timeout=10;logging=true";
for (int i = 0; i < 30; i++)
{
    MySqlConnection conn1 = new MySqlConnection(connectString);
    conn1.Open();
    conn1 = null;
    GC.Collect();
}
MySqlConnection conn2 = new MySqlConnection(connectString);
conn2.Open();

Console.WriteLine("Press enter to exit");
Console.ReadLine();
[16 Apr 2008 20:36] Robby Johnson
Hi,

Just curious if there have been any updates regarding this bug? 

Thanks
[12 Jun 2008 16:18] Gonzalo rafael Vazquez Cao
Hi, just out of curiosity, I've already have a workaround for this bug. But is it going to be fixed on further versions of the official release??

Thank you
[12 Jun 2008 16:22] Robby Johnson
How did you fix it, if you don't mind me asking? I'm trying to get a workaround for this as well.
[13 Jun 2008 10:07] Umberto Ballestrazzi
It takes less then 1h fix it.
There are several changes so if you fix it by your hands you will need to reapply the fix every new version.
Anyway, I still hope that someone at MySQL will fix it.

BTW, there's no good workaround, in case of unhandled exceptions the connection remains allocated but the driver will never use it again.
[13 Jun 2008 13:42] Robby Johnson
Hey Umberto, would it be possible for you to post the solution? Or even just a general description of how you did it?

I've been working with this bug off-and-on since January, and have yet to come up with anything.

And I'm sure there will be many others that would benefit from it, as it is a fairly significant issue for anyone who uses MySQL with Dot NET.

Thanks!
[14 Jun 2008 10:28] Umberto Ballestrazzi
Sure I can!

I post the files.
I reapplied the fix because I'm using an old version of the driver with some other behaviour changes (fix?) about date fields so please check it!

Actually I'm not sure about Dispose fix (at first test it works).
[14 Jun 2008 10:35] Umberto Ballestrazzi
Need to be tested! Use at your own risk!

Attachment: Connection Pool fix MySQL 5.1.zip (application/x-zip-compressed, text), 25.54 KiB.

[16 Jun 2008 13:11] Robby Johnson
Great stuff!

Thanks Umberto
[1 Sep 2009 7:53] Tonci Grgin
Same behavior observed using latest trunk.
[28 Sep 2009 9:00] Dan orsborne
Please fix this!! I spent weeks looking for the solution to this issue I was experiencing.
[23 Nov 2009 20:57] Vladislav Vaintroub
6.2 includes a fix for connection pooling. Unused connections that are idle for 3 minutes or more, are closed by periodically running timer job.
[24 Nov 2009 9:20] Umberto Ballestrazzi
Couldn't it be better to do like suggested? I think a timeout could not be the better way to solve the problem...
[24 Nov 2009 13:53] Vladislav Vaintroub
Umberto, I modeled unused connection cleanup after Oracle's with 3 minute unused timeout. And Microsoft documents similar behavior for clening up unused connections (without mentioning the timeout explicitely).
And we have to do periodic cleanups anyway, disregarding whether garbage collector runs or not, as unused pooled connections consume server resources, such as threads and sockets.
[24 Nov 2009 14:34] Umberto Ballestrazzi
Hi Vladislav, probably I did not understand.
Do you close every connection (still used or unused by the application) or only connection closed by the application but still opened in the driver?

Actually the problem was that the application must explicitly close the connection (the driver did not recognize that a connection was closed if the application did not call dispose or close method). If you don't put garbaged connections in the list of available connection (available to be closed after a timeout or reused) the implementation could not work.
[24 Nov 2009 19:43] Vladislav Vaintroub
Ok, I see what you mean. Since there was much talk in this thread about pooling, I misunderstood the issue. I reopen the bug, fix will follow soon.
[24 Nov 2009 19: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/91470

800 Vladislav Vaintroub	2009-11-24
      - Always close connection in MySqlConnection.Dispose(), also when it is garbage-collected
        so underlying driver can be reused in the connection pool (bug#31996)
[25 Nov 2009 16:16] 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/91653

715 Vladislav Vaintroub	2009-11-25
      - Always close connection in MySqlConnection.Dispose(), also when it is
      garbage-collected , so underlying driver can be reused in the connection
       pool (bug#31996)
[25 Nov 2009 16:27] 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/91656

777 Vladislav Vaintroub	2009-11-25
      - Always close connection in MySqlConnection.Dispose(), also when it is
      garbage-collected  so underlying driver can be reused in the connection pool (bug#31996)
[25 Nov 2009 20:32] 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/91710

805 Vladislav Vaintroub	2009-11-25
      - Always close connection in MySqlConnection.Dispose(), also when it is garbage-collected
        so underlying driver can be reused in the connection pool (bug#31996)
[25 Nov 2009 20:56] Vladislav Vaintroub
Pushed to 6.2 and trunk
[25 Nov 2009 21:09] Vladislav Vaintroub
Quite interesting that this simple fix did work only 6.2+

In earlier versions, for pooled connections, Dispose is never ever called. We likely store reference to connection globally, but I was not able to tell where exactly, after some hours of debugging.

@Umberto, if you know the fix for earlier versions, please post, patches welcome. Patches in form unified diffs (diff -u oldfile newfile), or "bzr diff" if you use bzr ,are really much appreciated and provide lot more insight into what was done (compared to complete zip of files or directories:)
[25 Nov 2009 22:10] Umberto Ballestrazzi
The problem is that the finalizer of the connection (~MySqlConnection) was not called by the garbage collector because of a reference from another object. You have to remove this reference (and probably implement the finalizer but I do not remember). To do this you need to change several files (but not deep changes).
Did you have a look to the zip files i sent some times ago? Inside you find only the files that you need to edit (really few changes).
If you compare them with the latest driver release i think you understand what you have to do (sorry but i do not program since i submit this bug :) ).

If you did not do this I think that the test program I sent (in the beginning of all posts) does not work even in latest version of the driver.
[26 Nov 2009 0:18] Vladislav Vaintroub
Umberto, I did not look at changes, they are against version 5.1 which we do not maintain anymore (and since then there have been a lot of changes, so the files do not look similar). Yes, in older version we probably maintain a reference somewhere and I'm not able

As for the latest changes, I added the test case that should prove it works. It passes currently, so I guess issue is resolved.

        public void ConnectionCloseByGC()
        {
            ConnectionClosedCheck check = new ConnectionClosedCheck();
            string connStr = GetConnectionString(true);
            MySqlConnection c = new MySqlConnection(connStr);
            c.StateChange += new StateChangeEventHandler(check.stateChangeHandler);
            c.Open();
            c = null;
            GC.Collect();
            GC.WaitForPendingFinalizers();
            Assert.IsTrue(check.closed);
        }
[26 Nov 2009 0:19] Vladislav Vaintroub
the latest example would be incompletee without ConnectionClosedCheck handler
here is the definition

        class ConnectionClosedCheck
        {
            public bool closed = false;
            public void stateChangeHandler(object sender, StateChangeEventArgs e)
            {
                if (e.CurrentState == ConnectionState.Closed)
                    closed = true;
            }
        }
[26 Nov 2009 2:38] 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/91722

806 Vladislav Vaintroub	2009-11-26
      Bug #31996 add-on  - remove reference to MySqlConnection from Driver, to help GC find unused
      connections first time through. Refactor MySqlField - it does not need ref on connection.
[26 Nov 2009 10:44] Umberto Ballestrazzi
Grate!!!

Just a question...
When you start to work on this bug you said that idle connections are closed after a timeout.
I do not understand what this mean...

Just to speak the same language i call Driver the low level connection to the server (the one that you enqueue for connection pooling) and MySqlConnection the connection used by the application.

If I have an application that opens a MySqlConnection and does not close it and does not use it for some time do you close it (and the associated Driver) after a timeout?

About closing Driver without a MySqlConnection associated with it after a timeout i think is a good idea.
Other cases could cause problems (also bad compatibility problems).
[26 Nov 2009 12:21] Vladislav Vaintroub
@Umberto, what I meant commenting on this bug about using timeout based cleanup , is following. In the past, if pooling was in use, the idle connections were never released, except explicitly with CleanPool. 

An application that opened say 100 concurrent connections at peak time, but uses 2-3 concurrent connections otherwise, would still maintain 100 open connections and consume server resources.

Timeout based cleanup is used to fix this situation. connections (or more precise Drivers) that stay idle/unused in the pool for long time (> 3 minutes) are closed.
[16 Dec 2009 13:20] Tony Bedford
An entry has been added to the 6.2.2 changelog:

Connection objects were not garbage collected when not in use.
[31 Jan 2010 7:51] Moshe Lampert
Fixed, but not always.
after a while I recived errors like this:

There is already an open DataReader associated with this Connection which must be closed first. 
at MySql.Data.MySqlClient.MySqlCommand.CheckState() 
at MySql.Data.MySqlClient.MySqlCommand.ExecuteReader(CommandBehavior be‚Äéhavior) 
at MySql.Data.MySqlClient.MySqlCommand.ExecuteNonQuery() 

I checked  if there a DataReader opened, but the error was in the first SQL query on the page.

Thanks for all!
[31 Jan 2010 9:55] Vladislav Vaintroub
Moshe, could you be a bit more specific? An example code would help.
[7 Apr 2010 6:43] Moshe Lampert
I am using the last version of the connector (6.2.2) and sometimes It returned a polled connection, that on the *first* SQL query return this exception.

I think It because the finalizer not close the resultset opened, when the connection is disposed/returned to the pool.