Bug #5022 Memory leak in ResultSet (close() does not release Fields)
Submitted: 12 Aug 2004 17:18 Modified: 20 May 2006 15:10
Reporter: Ken Johanson Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / J Severity:S1 (Critical)
Version:3.1 OS:Java
Assigned to: Mark Matthews CPU Architecture:Any

[12 Aug 2004 17:18] Ken Johanson
Description:
It appears that calling ResultSet.close() does not release (de-allocate) Fields stored in memory. This will manifest in an OutOfMemoryError on high volume servers if the programmer reuses Statement objects.

The problem does NOT occur if Statement.close() is called in addition to ResultSet.close(), however, it is a common occurance for programmers to only call ResultSet.close, perhps re-useing a statement.

My interpretation of the JDBC spec is that ResultSet.close is to "Releases this ResultSet object's database and JDBC resources immediately", and that this implies the fields should be released as well, without having to call Statement.close().

It also appears as though Statement objects that are not explicitly closed are not gc()'d, even long after falling out of scope. Unverified though.

An example hprof output showing the memory allocations can be downloaded from http://onnet.cc/java.hprof.txt

How to repeat:
Something roughly equiv to:

Connection con = <driver>.getConnection(DBCACHE);
try {
	Statement stmt = con.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_READ_ONLY);
	for (int i=0; i<1000; i++)
	{
		ResultSet rs = stmt.executeQuery(query);
		if (rs.next())
			;//ret = rs.getString(1);
		rs.close();
	}
	stmt.close();
} catch {
	..etc
[12 Aug 2004 17:48] Ken Johanson
My previous code example was poor at best, my aplogies ahead of time. Here is an improved version. Not the if the Statement declartion exists inside the for-loop, the problem exists. If is exists outide the loop, the problem does not occur...

Databases instance = Databases.getInstance();
Connection con = instance.getConnection("mysql");
try {
	for (int i=0; i<1000; i++)
	{
		Statement stmt = con.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE,ResultSet.CONCUR_READ_ONLY);
		ResultSet rs = stmt.executeQuery("SELECT * FROM phonlist");
		while (rs.next())
		{
			String s1 = rs.getString(1);
			String s2 = rs.getString(2);
			String s3 = rs.getString(3);
			String s4 = rs.getString(4);
			String s5 = rs.getString(5);
			String s6 = rs.getString(6);
			String s7 = rs.getString(7);
			String s8 = rs.getString(8);
			String s9 = rs.getString(9);
			if (i%200==0)
			{
				out.println((Runtime.getRuntime().totalMemory()/1024)+" "+(Runtime.getRuntime().freeMemory()/1024)+" "+rs.getString(1));
				out.flush();
			}
		}
		rs.close();
	}
	//stmt.close();
} catch (SQLException e) {
	out.println(Exceptions.getStackTrace(e));
}
instance.freeConnection("mysql", con);

Based on this test its becoming more apparent that there is not a working finalizer inside of Statement, or that it is being pooled..
[12 Aug 2004 23:53] Mark Matthews
I'm not sure exactly how to handle this. I've fixed the Field[] instance being 'held on to' by the result set for 3.1.4, however a finalizer in Statement (or ResultSet) for that matter is not the answer, at least in Sun's eyes. They're starting to discourage the use of finalizers as they intefere with Hotspot and the generational collector, impacting performance, witness the following article:

http://www-106.ibm.com/developerworks/java/library/j-jtp01274.html

If you're not calling close() on statements, as far as JDBC is concerned, it is your application that is 'leaking', and there's not a lot that a JDBC driver can do (especially once Statements can be backed up by server-side cursors on MySQL).

Does this issue occur if you take care of calling .close() appropriately in your code?
[13 Aug 2004 0:30] Ken Johanson
When I call close on Statement inside of each iteration, all is okay. ResultSet.close though, did not free the memery.

I agree that it should be possible to release the memory without use of a finalizer. But I'm not sure I agree that calling Statement.close is a rule for freeing ResultSet fields, nor that statement shouldnt be eligibe for GC if noone's holding a reference to it...

Glad to hear theres a patch already!!! :-) Can I "borrow" it for testing, please?? :-)

Thanks!
ken
[13 Aug 2004 3:54] Mark Matthews
See tonight's nightly snapshot build from http://downloads.mysql.com/snapshots.php if you want to test this.
[14 Aug 2004 4:03] Ken Johanson
Mark, I'm still seeing the leak. ResultSet.close still isnt freeing the Fields.
[14 Aug 2004 5:31] Mark Matthews
I see that the change actually didn't get pulled from a merge of 3.0. This will be fixed for tomorrow's snapshot build. Sorry for the inconvenience.

I still do find it puzzling however that Field[] instances occupy enough memory even without this fix to cause an out-of-memory condition unless the GC isn't getting to your abandoned Statement instances until it is too late to recover enough memory for your program to keep running.
[15 Aug 2004 3:57] Ken Johanson
Today's build is much better, I'd say almost 1/4 or 1/8 the rate of memory consumption.. but there's still some happening.

http://onnet.cc/java.hprof2.txt was generated with the java arg Xrunhprof:heap=sites,depth=10,doe=y

and the following (simplified version) loop:

try {
	Statement stmt;
	for (int i=0; i<10000; i++)
	{
		stmt = con.createStatement(); 
		ResultSet rs = stmt.executeQuery("SELECT * FROM phonlist");
		while (rs.next())
		{
			if (i%200==0 && rs.getRow()==1)
			{
				out.println((Runtime.getRuntime().totalMemory()/1024)+" "+(Runtime.getRuntime().freeMemory()/1024)+" "+rs.getString(1));
				out.flush();
			}
		}
		rs.close();
	}
	//stmt.close();
} catch (Throwable e) {
	out.println(e.toString());
}

Most of the memory still appears to belong to com.mysql.jdbc.ByteArrayBuffer and com.mysql.jdbc.Field..

I dont suppose this could be related to the fact that in a connection-reuse/pooling environment, a continued reference to Connection is held by Statement (ala .getConnection()), and because Statement also keeps a ref to ResultSet, preventing gc?... in this case it seems almost mandatory that Statement.close be called, or that a pooling environment should actually pool/reuse Statement objects, not just Connection -- or that a finalizer be used in Statement to null its Connection ref. Just thinking..
[20 Oct 2004 15:01] Ken Johanson
Mark, If I may ask, what was the fix?
[9 Dec 2005 13:11] Colin Garriga-Salaün
Excuse me but that bug does not seem to be fixed. I am working on an application using aggressively the connector/J. Well, it appears that I had a memory leak with ResultSet and Field not being freed despite of calling the close() method. The heap grew up and crashed the application.

I fixed my problem switching from version 3.1.12 to 3.0.17. Significant, isn't it?

Sincerly,
Colin
[9 Dec 2005 13:43] Mark Matthews
Carlos,

If you look at the code involved, there is no way fields can't be released (see ResultSet.realClose()).

Since this fix has been made, the only time we've seen memory leaks is when someone missed some code path in their application code and they weren't calling .close() when they thought they were.

A quick test is to add "dontTrackOpenResources=true" to your JDBC URL. If the memory leak goes away, some code path in your application isn't closing statements and result sets.
[12 Dec 2005 13:00] Colin Garriga-Salaün
I didn't made any modification in my own code. I have just changed the connector that's why i am reporting it right now. The code I wrote is structured like reported below. It might be not an optimal or a very idiomatic code style, but I hope that it is strong. With 3.1 connector it leaks whereas with 3.0 it does not.

I hope this will be hopefull. If you are not convinced yet I'll try to investigate the connector's code myself. But I have not much time.

Sincerly,
Colin.

<code>
Connection connection = null;
PreparedStatement statement = null;
ResultSet results = null;

try {
	connection = this.getConnection();
	statement = connection.prepareStatement(...);
	...			
	results = statement.executeQuery();
        ...
} catch (...) {
	...
} finally {
	if (results != null) {
		try {
			results.close();
		} catch (SQLException ex) {
			String msg = "...";
			log.warn(msg, ex);
		}
		results = null;
	}
	if (statement != null) {
		try {
			statement.close();
		} catch (SQLException ex) {
			String msg = "...";
			log.warn(msg, ex);
		}
		statement = null;
	}
	if (connection != null) {
		this.returnConnection(connection);
		connection = null;
	}
}
</code>
[12 Dec 2005 13:03] Colin Garriga-Salaün
.. and I will look at the "dontTrackOpenResources" option as soon as possible.
[10 Jan 2006 15:57] Ken Johanson
Mark, this is the orig. reporter, here...

This bug is back on the latest drivers. I'm using a mysql-connector-java-3.1-nightly-20051230-bin.jar, though now IF (only) the resultset is closed, no memory leak is occuring (anything obvious anyway).

However if the resultset is not closed, the system will eventually suffer an unrecoverable OutOfMemoryError.

While explicitly closing ResultSet/Statement is the correct way, it is not expectable (due to programmer error or laziness), and these points are especially poiniant in a hosting environment where 3rd parties (possibly having malicious intent) can place code on a shared server and effect denial of service. In any case, when no references (in the user application) are being held to the objects, they should be free-able by GC (the essence of a 'managed' system like java).

The spec for *.close() says "immediately release resources", which implies that the GC would (should be allowed-to) otherwise collect them.

I concur with the last commentor; this problem did not occur on earlier versions - only on the latest builds.

Raising to 'critical' since this has DOS implications for hosting providers.
[10 Jan 2006 16:10] Mark Matthews
Sorry, but there is no transparent workaround if we are to be JDBC-3.0 compliant (and still perform well), which requires that until closed, connections keep track of non-closed statements, and statements keep track of non-closed result sets. 

JDBC-3.0 states that drivers can use a finalizer to clean up non-closed resources, although JDBC-4.0 will prohibit it (as most vendors don't do it, because it's a 15-20% overhead across the board performance-wise), and in practice causes deadlocks. Using weak references for the same thing also causes a similar overhead, and you still have issues with deadlocks because you're interfacing with a system outside the JVM which has it's own ideas of order of operations that can't be enforced when you're relying on non-deterministic GC and reachability to determine when to close resources.

If you _don't_ want JDBC compliance, you can always add "dontTrackOpenResources=true" to the MySQL URL and although the driver will not act in a JDBC-compliant way, you won't see these "memory leaks" that are caused between a mismatch of the JDBC specification and how your application behaves.
[19 May 2006 19:55] Jose Bonetti
PROBLEM WITH MYSQL DRIVER AND MEMORY LEAKS FIELD[]

Its true there is problems with com.mysql.jdbc.Field[] it doesnt release THEM even if you close the Result Set AND stament. 

This is was giving me a lot of headaches on a connected application i have, going out of memory in a few hours.I followed some forum advice and i'm currently using THE 3.0.17 DRIVER and the Fields[] are disposed fine.

Mark, if you want PROFILER TESTS FROM THE APPLICATION , i can send you.
jose.bonetti@gmail.com

Jose Bonetti
Soluciones GBH.
[19 May 2006 20:02] Mark Matthews
Jose, if you have profiler results, we'd appreciate them. However, what version of the driver are you using exactly? Can you test with a nightly snapshot of 3.1.13 (http://downloads.mysql.com/snapshots.php#connector-j)

We've not seen this in our testing, and we have quite a few people in production with it, so we'd like to know if we're missing some execution path, or if people are reporting leaks that we've already fixed.
[19 May 2006 20:18] Jose Bonetti
Mark,

I did Jprofiler on it with your latest driver, and the driver before that, and my application didnt last more than 3 ours, til it overran all the memory , the profiler showed me the Fields[] was the hot spot, rising over 3000, in 4 hours.

I followed ome posts that lead me to 3.0.17 driver and know it removes then properly.

I will send the profile tests.

att

Jose
[19 May 2006 20:22] Mark Matthews
Jose,

I need to know what you mean exactly by "latest". It's a very vague term. Please use _exact_ version numbers and filenames. 

Thanks,

  -Mark
[19 May 2006 20:25] Jose Bonetti
3.1.12 // its currently available here 

http://dev.mysql.com/downloads/connector/j/3.1.html
[19 May 2006 20:26] Jose Bonetti
Im currently creating the SNAPSHOTS for the 3.0.17
then i'll procede to do a 3.1.13 and 3.1.12(again)

so you can see the difference
[19 May 2006 20:34] Mark Matthews
Testing 3.1.12 from http://dev.mysql.com/downloads/connector/j/3.1.html isn't helpful. The bug fixes in question are in the source repository, which is why I'm asking you to test a nightly build.

Please test this _precise_ version:

http://downloads.mysql.com/snapshots/mysql-connector-java-3.1/mysql-connector-java-3.1-nig...
[19 May 2006 20:37] Jose Bonetti
3.1.12 is the only visible driver in the site, i didnt even know 3.1.13 existed
i've now download it and will perform the tests where can i upload the outcome on the tests i have screenshots from the profiler
[19 May 2006 20:37] Mark Matthews
Jose,

If you look at the code in question, in ResultSet.realClose(), the references to Field instances are set to null in a finally {} block, other than running out of memory, there's no way they're leaking from there that I can see.

The only other potential for leaks _I_ can see is if an _application_ uses ResultSetMetaData and leaks instances of those itself.
[19 May 2006 20:47] Jose Bonetti
As you may understand i'm not testing with the code in hand, this is based on an application i've developed, which is in production and this same specific bug is causing the leaks, so you can't use ResultSet.close instead u need to use realClose() for it to finish it ? .

Again where can i post the screens?
[19 May 2006 20:56] Mark Matthews
ResultSet.realClose() is private. It's called from .close(), so if you're calling .close(), it's being called.

You can attach the profiler data to this bug report via the "Files" tab.
[20 May 2006 3:52] Jose Bonetti
1rst/2nd file File consists on tests results based on 3.0.17
--------------------------------------------------------------------
Working perfectly (this version)

regular increase
http://www.gbh.com.do/3.0.17increasing.jpg

regular decrease(what it should do in every driver
http://www.gbh.com.do/3.0.17-screen1 decreasing

2nd/3rd results on 3.1.12 
------------------------------
Just keeps growing picture 1 and 2
http://www.gbh.com.do/3.1.12-sametime-sameload-increase.jpg
http://www.gbh.com.do/3.1.12-just-keeps-growing.jpg
4th result on 3.1.13 , which it was even worse
-----------------------------------------------------
http:/www.gbh.com.do/3.1.13-even-worse.jpg

Am performing the same program, same uptime time, see for yourselves.
performing my stament close and resultset close everywhere is supposed to be  proving that 3.0.17 RELEASES the fields and the other ones just doesnt.

Thanks for the prompt help on this matter

Jose Bonetti
[20 May 2006 3:57] Jose Bonetti
Excuse me:

Soory the second link is

http://www.gbh.com.do/3.0.17-screendecreasing.jpg
[20 May 2006 14:14] Jose Bonetti
As you can see, Field growth generates String , byte , char arrays which make the leak even larger, 'cause of the non stop object growth on com.mysql.jdbc.Fields.

I dont specifically know WHERE in the driver code, the mistake is, but you guys should version check the 3.0.17 and compare where did it go wrong on the ResultSet close, and Field nullin' cause the objects are up and running, what is the test envoirment of the developer team ?  I could take a look at the code if i could colaborate, but might take me a few days to look at the API etc.

Best Regards,
Jose Bonetti
[20 May 2006 14:39] Mark Matthews
Can you do an object allocation backtrace? We haven't seen this leak in our own testing, nor in third-party benchmarking, so there's a good chance it's related to a particular use case that we're not testing for.

Like I stated before, the only place Field[] instances are referenced are in ResultSets and ResultSetMetaData, and ResultSets themselves can not theoretically leak them if .close() is called on them, as the references are set to NULL in a finally block that _always_ executes if .close() is called.

Have you tested with "dontTrackOpenResources=true"? If that makes the problem go away, then there's a good chance there's a leak in application or framework code somewhere that you're not aware of. Before we debug this further, I'd _really_ like to rule out that this isn't a leak somewhere outside of the JDBC driver.
[20 May 2006 14:42] Mark Matthews
As far as the development environment, we use Eclipse ourselves, but you get the sources with the driver you download.

The difference between 3.0 and 3.1 entirely has to do with tracking open resources. Nearly every time we've seen this (except for one time, the original opening of this bug report), the issue has been unexpected behavior in the _application_ causing ResultSet.close() not to be called.

Feel free to take a look at com.mysql.jdbc.ResultSet.close(), and .realClose(boolean), and see if you can figure out any way, short of an OutOfMemoryError (which is nearly fatal anyway) that the reference to ResultSet.fields can not be set to null. I myself can't see it.
[20 May 2006 14:49] Mark Matthews
> Am performing the same program, same uptime time, see for yourselves.
> performing my stament close and resultset close everywhere is supposed
> to be  proving that 3.0.17 RELEASES the fields and the other ones just
> doesnt.

One comment on the above statement...Running with 3.0.17 can't prove you're calling .close() _everywhere_ on result sets. In Connector/J 3.0.x, Statements don't hold on to references to not-closed result sets like they do in Connector/J 3.1 (a behavior required by the JDBC specification), it just means they go out of scope automatically when no longer referenced by the application, and are thus eligible for GC. 

That is why I've asked you to test with "dontTrackOpenResources=true", because that gives the behavior of Connector/J 3.0.x. If running with that causes the leak to go away, then there's a 99% chance the leak is somewhere outside the JDBC driver.
[20 May 2006 15:00] Jose Bonetti
Just used dontTrackOpenResources on the app, and now it's even more notorious that com.mysql.jdbc.Field 
and com.mysql.jdbc.ByteArrayBuffer are scaling without  any release whatsoever.

i'll test with a later driver and this same option.
[20 May 2006 15:03] Jose Bonetti
This is scenario am having a Persistent connection to a database and the program runs 2 TimerTasks that querys mysql dbase on n seconds.

select from a remote mysql dbase 
get the new calls

show them on screen

save the call on local dbase
as it is a new call dont look for it as a new call nomore, just look to see if it has ended

remote connect to see if it has ended

if it has ended
save in dbase it has ended
[20 May 2006 15:08] Jose Bonetti
Ah thanks for the aclaration on donttrackopenresources.
[20 May 2006 15:10] Ken Johanson
Jose, can you provide some very simple sample code that will demonstrate the leak? Perhaps like I did so on my initial report... that makes conclusive proof of/not a leak, that's easy to verify.. apps on the other hand have too many variables (pools, concurrent connections).. 

I too am interested to know if there is a problem, but I think your best case will be made in the form of sample code that we can run too.

Thx,
ken
[20 May 2006 15:17] Mark Matthews
> Just used dontTrackOpenResources on the app, and now it's even more
> notorious that com.mysql.jdbc.Field 
> and com.mysql.jdbc.ByteArrayBuffer are scaling without  any release
> whatsoever.
> 
> i'll test with a later driver and this same option.

Logically, I don't see how that is possible. I think there's something going on more complex here than a simple leak inside the driver. As Ken asks, can you boil this down to a simple repeatable example? We have large deployments in the field running 3.1.12 and 3.1.13 (especially for large scale benchmarks), and if the leaks were this serious for the use cases most people are dealing with, we would've heard about it before now.

Either your application is tickling a rare bug, or is hitting some portion or combination of the driver's code that neither our tests or our users usually hit.

In general, without either access to your system, or a reproducible testcase, these things are hard to diagnose.
[20 May 2006 15:18] Jose Bonetti
Register calls 

http://gbh.com.do/mysql/IngresoLlamada.java

End calls

http://gbh.com.do/mysql/TerminoLlamada.java

Timer Task deamon

http://gbh.com.do/mysql/Verificador.java

soory for the spanish ;)
[20 May 2006 15:24] Jose Bonetti
Mark, i can give you acces to my application through logmein or some sort.

If there anyway to chat even quicker we could settle it
msn: jbonetti_lugo@hotmail.com
gmail: jose.bonetti@gmail.com
[20 May 2006 15:39] Jose Bonetti
Please remember this is a Peristent connection, to both local and remote locations

Heres the Connection class

http://www.gbh.com.do/mysql/Conexion.java

Where the connection its instantiated

http://www.gbh.com.do/mysql/Configuracion.java

and the manager thread

http://www.gbh.com.do/mysql/ConfiguracionThread.java
[22 May 2006 13:56] Jose Bonetti
Have you been able to test or look at the code, what about the app access, when can we coordinate that?
[22 May 2006 14:01] Mark Matthews
I've taken a look at your code, and see quite a few places where statements and/or result sets could leak. For example, in any exceptional condition, you only set a value to NULL, which doesn't close it, it just makes it eligible for GC. You should call .close() in finally blocks as well.

Generally, we can't do remote access to users systems unless a support contract is in place for legal reasons (liability, in general). Can you create a standalone testcase that demonstrates the behavior you're seeing so we can look at it locally with our tools?
[22 May 2006 15:13] Jose Bonetti
Yeah i think i can recreate this, to stand alone, i'll post the link tomorow with the modified app.

Btw Files tab only work for "developers" so i'll post again on my server

Tnx

Jose
[26 May 2006 21:04] Jeff Ash
I was experiencing the same issue with a memory leak, only on large bulk operations and finally tracked it down to a place in my code where I was not explicitly closing a PreparedStatement nor the ResultSet.  Once I did this, everything works great now.