To close or not to close that is the question
Duncan Wild
jdbc and resultsets, its all very simple stuff no ejb or fancy stuff
its just using resultsets to get the data. Its been standard practice
that when you open a resultset you then close it in a finally block to
avoid any memory leaks.
Now the reason its standard practice at our place to close resultsets
that you opened, is because we had this program which extracted data
from the DB and it ran a couple of thousand queries on some of the
larger datasets that the clients have. What we experienced was it
would get so far and get the out of memory exception, and the way we
fixed it was to hunt down all the resultsets which weren't being
closed, this was about two years ago...
The other day this new guy told me that you dont need to close
resultsets because the GC will come along when memorys getting low
and will close them for you and tidy everything up.
Also when I worked with a company starting with the letter "O" and
ending in "cle" for a short while I noticed that they also never
bothered closing the resultsets, when I asked the programmer who wrote
it why not he said that when you close the connection then the
resultset will be closed and there is no reason why you should bother
closing them beforehand, but they were experiencing "maximum open
cursors exceeded" errors in the app and didn't know why, although they
were adamant it wasn't due to not closing resultsets...
So my question is, do you actually need to close the resultsets or
not ?
the app is running in 1.3.1 talking to a SQL DB if that makes any
difference...
Casper Bang
large master-detail chains causing an excessive amount of cursors to
be opened. Our DBA told me that maintaining cursors is quite expensive
for the DBMS so I now always use the following idiom and have since
using this, not seen any memory or cursor exceptions. So while the GC
might kick in and close result sets eventually, if you have large
nested queries that looks almost like a functional mapping from method
name to table name (getCustomer on Customers) cursor use grows
exponentially, so always close the result set or let the DBMS do more
of the work (fetch larger result sets and extract into key-value maps
through nested loops in Java). I am unaware of how EJB3/JPA behaves in
regard to open record sets and cursors, but I am pretty sure it closes
its record sets in the same fashion as below.
PreparedStatement stmt = connection.prepareStatement("SELECT ...");
try
{
// Bind Java to SQL binding variables
ResultSet rs = stmt.executeQuery();
try
{
while(rs.next())
{
// Handle record
}
}finally{ rs.close(); }
}finally{ stmt.close(); }
/Casper
Rich Liu
Since it's inexpensive to test for null on the connection, statement, and result set, we've made it a standard practice to close all of them just to be safe. We err on the side of defensive programming.
We have not had issues with our database resources since.
Stephen Kuenzli
blocks when done with them.
Spring's JdbcTemplate appears to do this too:
http://springframework.cvs.sourceforge.net/springframework/spring/src/org/springframework/jdbc/core/JdbcTemplate.java?revision=1.136.2.1&view=markup
Anyone know what FindBugs says about not closing JDBC resources when
done with them?
Stephen
tau...@gmail.com
Statements would get closed. JBoss's pooling has some code to close
things for you if you didn't. In any case I think it is always a good
idea to close things yourself.
-Dave
Dick Wall
would strongly recommend always closing results sets as soon as you
are done with them. We used to create reset() methods on any classes
that regularly used results sets to do their work, which would close
out any existing results sets and set the references back to null (so
people couldn't try and use old ones).
Channeling my friend Mr. Carl Quinn here as well, I would remind you
that the Java spec doesn't give any promises of when or even if an
object will be GC'd and finalized. If you have a long running
application, and GC doesn't happen to kick in for that object, it
could be held open indefinitely. Most shops I have worked (the current
one included) have rules about putting any kind of cleanup in a
finalizer, and recommend always using try-finally to clean up after
yourself instead.
I personally believe the O*cle folks who were adamant about the cursor
limit not being a result of too many open results sets was probably in
a bit of denial. I don't know their implementation specifically, but I
would have thought this was almost certain to be the cause. Either
way, cleaning up results sets when they are finished with sounds like
a very good idea.
Cheers
Dick
Christian Catchpole
I dont think GC is relevant because state is still maintained to the
database and much of the state may not be collectible, even if your
code has no references. A friend of mine wrote a finally method into
one of his utility classes which sent big error messages to his
developers that they had not released resources properly (because they
were causing resource leaks - and yes, it started with O and ended
with icl). They kept complaining about this message. He said, "well,
go fix your code and it will go away".
So, (that's an official Posse 'So'), even if some driver to some
database SEEMs to be working without close, it may not work in a
subsequent driver version, another driver, or another JVM. Connection
pools can hide you a bit from the real implementaton, but at the end
of the day, you can never realy be sure.
Its funny how people swear by things because they seem to work. We
get a free lunch with GC for the most part. It's not too much to ask
to close the resources which say they require it. :)
Tobias Frech
The JDBC spec needs the JDBC driver to free the resources once it is
garbage collected. Some (at least one big) vendor simply ignore this
requirement because they have to support some border mainframe cases
as well. That's why in reality you always need to close your stuff at
some point in time yourself. FindBugs is a valueable tool in this area
as well.
Cheers,
Tobias
Christian Catchpole
i said gc wasnt relevant. thats not realy what i meant. i was trying
to suggest you shouldnt depend on it. even if finally does close
connections, a production system may have lots of memory and many
successive jdbc calls may leave too many open connections waiting to
be collected. something you may not notice in test. as mentioned i
have seen this happen on production systems.
Christian Catchpole
connections and supply virtual connections to the user (connections
which dont realy open and close at a JDBC level). This is for
performance and also to deal with prepared statements (which are
standard practice in non-trivial systems). Prepared statements are
assocaited with a real connections which are lost when closed. So a
pool will try to match a connection with an established prepared
statement if you prepare that statement again. Why am i mentioning
this? Calling close will put that connection back in the pool and
will allow it to be recycled quick smart. So it's just not about
total number of real connections, is about helping your connection
pool give you that primed connection back.
Jack
1. New guy at your company saying there's no need close database
resources because garbage collection will take care of it.
In my experience, this is a terrible practice. It assumes GC will get
invoked before you run out of database resources, which is not always
a valid assumption.
2. O*cle guy not closing ResultSets because Connection#close() calls
closes them.
This is not as bad, in my opinion, but I still wouldn't do it. Even if
your JDBC Driver were 100% compliant, you aren't necessarily
interacting with the driver classes directly (e.g., you may be
accessing them through delegates provided by your connection pool).
Your actual implementation may defer ResultSet closing, for instance.
Too much can potentially go wrong.
BTW, can we really trust a company that gave us "VARCHAR2" and
"DUAL"? ;)
On Jul 26, 8:38 am, Duncan Wild <duncan.c.w...@gmail.com> wrote:
Christian Catchpole
I've seen developers say that String == String is valid if the Strings
originated from the same constants. thats just playing with fire.. i
think it was just an excuse. :)
andrew.bruce.law
related to JDBC, but I'll continue as it reinforces all the points
already made.
A long time ago, on a large project on which I was a consultant we had
a memory leak. You;d only see it in soak testing when you ran loads
and loads of messages through (it was very heavy on the MQ) and
eventually the testing guys found that the JVM process size would
increase by ~8 bytes per message received. It didn't vary with
message size. What was going on? We turned our profiling tools on it
so we could see where the memory was going but found nothing. "How
could this be?" the customer thought. "Java can't memory leak". It
didn't help that At this time I was working for the folks who had
invented Java (starts with "S" and ends in "un"if you need help with
that one) and so the finger of blame as far as they were concerned was
pointing firmly at us.
To cut a long story short we spent ages looking for it. The customer
was convinced it was in our (S-U-Ns) code. It got very political and
all the while we still couldn't find the source, until one day we
found it, quite by accident. You'll have guessed that it was a
resource which was being opened and then never closed; in our case
ZipInputStream. What they'd done was to use this to get access to
some info from a file each time a message was received. Problem was
in their code they'd never closed the reader once they were done.
(Note we'd asked them if they had always closed things they opened and
they swore blind they had) "No problems, It'd be collected by GC
after it was freed at the end of the try block right?" Errrr right-
ish. It turns out, much to their surprise, that GC can only clean up
Java pieces. If there is native code (such as there was here to access
the native filesystem), this will be left behind; unless you give the
API the chance to clean it up for you. Forget and you'll leak C code
within your JVM process but have a darn hard time finding the source
of it.
The moral of the story? Always clean up after yourself. Especially if
there is a native component to your API (JDBC drivers anyone?)
Regs, Andrew
Jess Holle
Isn't it best practice for anything that has native code cleanup to have a finalizer somewhere in the mix that will eventually do this. You should still do the explicit cleanup, of course, so that this stuff can be cleaned up in a quick, efficient, and timely manner (whereas finalizers don't really meet any of these criteria), but the finalizer safety net should still be there. If for some reason the finalizer is too expensive (they're far from free), then at least the API should have a huge Javadoc warning stating the severe consequences of not cleaning up and explaining that the expected finalizer is missing as making those who do cleanup properly pay the performance penalty was deemed inappropriate, unfair, etc. I believe the vast majority of cases shouldn't force one to choose between good (not perfect) performance and safety, though -- in which case safety shouldn't be sacrificed.
On a related note, we spent a *long* time trying to track down a horrific memory leak in a customer system. It appeared many critical class' finalizers were not being run *at all* (I'm not talking running slowly, I'm talking never). It was eventually tracked down to a horrible bug in another JVM vendors 1.4.2_12 release. They had a 1.4.2_13 that addressed the problem, but the only mention of this critical stealthy bug was buried in the release notes for one of their 1.5.0_xx release updates.
Moral of the story for JVM vendors: Loudly own up to any serious issues when you have fixes available [i.e. make this easy to find on the web site, a prominent part of fixed releases' release notes, etc] and urge your customers to move to the fixed version. Your customer, ISVs, etc, will be much happier with you in the end.
Overall moral of the story for the rest of us: When troubleshooting, trust no one, but trust yourself least of all. Again you'll come off much better in the end (as opposed to blaming bugs on others only to find out they're yours).
--
Jess Holle
Reinier Zwitserloot
resources are pretty heavyweight anyway, so there's no issue there,
but for lightweight objects you really don't want a finalizer.
modern java VMs use an Eden type garbage collectors: A single smallish
block of memory is where newly minted objects start their lifecycle.
This is 'eden'. Once eden is full, only those objects which are still
referenced someplace get copied into the proper heap, and then eden
gets reused. Whichever objects no longer held references are thus
silently overwritten. This makes eden type garbage collecting work
faster than C code - in C code, calling the deconstructor takes some
time, whereas in java 95%+ of all objects deconstruct in zero time.
Because finalization imposes extra work, anything anywhere that has an
explicit finalizer must always graduate from eden. For small
frequently created objects (like strings and other small fries), this
is a huge performance hit.
Database connections are probably far too heavy weight to worry about
the eden penalty.
-However-, I'm virtually positive JDBC resources all already use this
technique - their finalizers clean them up. The problem is, in a java
VM running with a large heap, or written in such a way that it doesn't
create all that many objects that graduate eden but then get discarded
soon after that (THOSE are the objects that the garbage collector will
actually clean up. Eden objects get cleaned up so fast, you never
notice) - actual garbage collection and the related finalization can
take many minutes.
I don't mean to sound condescending - from the tone of your post I
think you already know all this. Just clearing up some things for
interested readers who don't know this stuff.
> a finalizer somewhere in the mix that will /eventually/ do this. You
> should /still/ do the explicit cleanup, of course, so that this stuff
> can be cleaned up in a quick, efficient, and timely manner (whereas
> finalizers don't really meet any of these criteria), but the finalizer
> safety net should still be there. If for some reason the finalizer is
> too expensive (they're far from free), then at /least/ the API should
> have a huge Javadoc warning stating the severe consequences of not
> cleaning up and explaining that the expected finalizer is missing as
> making those who /do /cleanup properly pay the performance penalty was
> deemed inappropriate, unfair, etc. I believe the vast majority of cases
> shouldn't force one to choose between /good /(not perfect) performance
> and safety, though -- in which case safety shouldn't be sacrificed.
>
> On a related note, we spent a *long* time trying to track down a
> horrific memory leak in a customer system. It appeared many critical
> class' finalizers were not being run /*at all*/ (I'm not talking running
> slowly, I'm talking /never/). It was eventually tracked down to a
mikeb01
absolutely correct. The purpose of the GC is to manage memory and not
any other sort of resource (files, DB connections, etc.). You should
close any resources that you open, its just good practise. In a Java
EE environment connections are not actually closed. Some JDBC drivers
hang on to references to ResultSets and Statements (O*cle does),
therefore those objects will still navigable as far as the GC is
concerned and won't get cleaned up. This definitely leads to "Max
open cursors exceeded" errors (having had to track down and fix such
errors). As a small addendum, you should always swallow exceptions
that occur when closing connections, statements or result sets. If
you consider the following code:
try {
Connection cn = ....;
Statements st = cn.createStatement();
ResultSet rs = st.executeQuery();
} finally {
rs.close();
st.close();
cn.close();
}
If the rs.close() throws an exception, the Statement and Connection
will not be closed (leaks will occur) and you will lose the original
exception (really bad and expensive if it happens in production). So
I normally do:
// Repeat for Statement, Connection
void close(ResultSet rs) {
if (rs != null) {
try {
rs.close();
}
catch (Exception e) {
e.printStackTrace();
}
}
}
try {
Connection cn = ....;
Statements st = cn.createStatement();
ResultSet rs = st.executeQuery();
} finally {
close(rs);
close(st);
close(cn);
}
Mike.
Christian Catchpole
I think finalizers are used as a last resort clean up method. This
does not imply they can be depended apon. Kind of like on aircraft..
just because they have life jackets doesn't mean they shouldn't put
the landing gear down. :)
Marcus Olk
> void close(ResultSet rs) {
> if (rs != null) {
> try {
> rs.close();
> }
> catch (Exception e) {
> e.printStackTrace();
> }
> }
> }
Bummer there's no common interface such as java.io.Closable, huh?
Due to the fact that SQLException and IOException are checked exceptions
it's impossible to use java.io.Closable here - bloody checked exceptions...
Hence we're using a 'duck type variant' using reflection invoking
'a Close() method' on a given Object here. Kinda Groovy :)
Marcus
Jess Holle
One should always close(), etc, etc, as the repercussions of not doing so are severe.
That said, finalizers should be there as a safety net when not having them results in a permanent native memory leak.
In the 1.4.2_12 case (actually 1.4.2.12 in that vendor's case as I recall), the issue was that finalizers weren't getting called properly for some of our heavy classes which depend upon finalizers for some cleanup. You point out lots of reasons this is not a good thing to do, but there is a time and place for such -- and it works quite well except on that JVM.
--
Jess Holle
Reinier Zwitserloot
quitting VM -- eventually.
Java is not allowed, during the continued running of a VM, to avoid
running finalizers when objects are tossed. However, it is allowed to
keep a collectable object in memory (and thereby, never finalize it)
indefinitely. This makes absolutely NO difference from the
programmer's perspective in the behaviour of finalizers (specifically:
Your current opinion that finalizers are an optional thing you should
never rely on is for all practical purposes true), but once you get
into VM nitty gritty, the distinction is important. In this case,
while they are optional from the standpoint of a programmer, they are
totally NOT optional from the standpoint of a performance analist. A
finalizer will make GC of the specific object anywhere from a little
slower (cost of running it) to a whole lot slower (cost of enforced
unneccessary graduation from eden).
NB: the term 'graduation' is something I made up. There's another word
with similar meaning that's commonly used in discussions but I forgot
what it was. Just an FYI for those trying to google this stuff. 'eden'
is the official term for this scheme though.
Reinier Zwitserloot
parameterless close() method with reflection, call it catching all
exceptions and just returning the exception or null, because in 90% of
the cases I don't care about a close() op not working), or rather
copied it into new projects, a heck of a lot :/
Christian Catchpole
just talking in terms of closing DB connections - because of the
reasons of potentialy many open connections waiting to be finalized.
Same story will file handles etc. This wasn't a criticism of
finalizers per se.
Rick
http://java.sun.com/javase/6/docs/api/javax/sql/PooledConnection.html
Has some interesting stuff to say about the javax.sql.PooledCennction
If I'm reading it right, it is basically saying that you should
_always_ call close() on your connections and prepared statements, but
that if you somehow manage to call the wrong close() then all heck
will break loose.
Christian Catchpole
My code base has a net.catchpole.lang.Disposable interface. It
originated in my custom SWT widgets (Fidgets :) but I soon realized it
would be very handy a core level interface for anything which requires
explicit disposal. No ducks involved though. :)
Marcus Olk
> originated in my custom SWT widgets (Fidgets :) but I soon realized it
> would be very handy a core level interface for anything which requires
> explicit disposal. No ducks involved though. :)
Yep. Right. Got the same class as well - the ever lasting 're-invention-of-the-wheel'.
It's obvious that one needs such an interface (SWT Disposable and JDK Closeable).
But it seems it's not as obvious as required.
No ducks involved? And what is the dynamic proxy all about if not ducks? :)
Marcus
_______________________________________________________________________
Jetzt neu! Schützen Sie Ihren PC mit McAfee und WEB.DE. 3 Monate
kostenlos testen. http://www.pc-sicherheit.web.de/startseite/?mc=022220
Christian Catchpole
Actually I wasn't even aware of the JDK1.5 Closable until now. It
throws IO Exception but the method name close() seems to be an
accepted standard for such things.
Without wheel reinventions we would have chrome alloys. :)