MySQL Bugs: #31996: Connection pooling - MySqlConnection object release

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

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).

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?

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).

Ops...
I forgot the version...
The newest source code date is 31/08, probably 5.0.8.1

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.

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

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

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.

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

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.

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

Hi,

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

Thanks

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

How did you fix it, if you don't mind me asking? I'm trying to get a workaround for this as well.

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.

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!

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).

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.

Great stuff!

Thanks Umberto

Same behavior observed using latest trunk.

Please fix this!! I spent weeks looking for the solution to this issue I was experiencing.

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.

Couldn't it be better to do like suggested? I think a timeout could not be the better way to solve the problem...

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.

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.

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.

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)

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)

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)

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)

Pushed to 6.2 and trunk

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:)

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.

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

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

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.

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).

@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.

An entry has been added to the 6.2.2 changelog:

Connection objects were not garbage collected when not in use.

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!

Moshe, could you be a bit more specific? An example code would help.

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.