Hi Stefan
I would try this one:
Connection conn = null;
Statement stmt = null; // Or PreparedStatement if needed
try {
conn = ... get connection from connection pool ...
stmt = conn.createStatement("select ...");
ResultSet rs = stmt.executeQuery();
... iterate through the result set ...
} catch (SQLException e) {
... deal with errors ...
} finally {
// Always make sure result sets and statements are closed,
// and the connection is returned to the pool
if (stmt != null) {
try { stmt.close(); } catch (SQLException e) { ; }
}
if (conn != null) {
try { conn.close(); } catch (SQLException e) { ; }
}
}
We don´t need to null conn and stmt variables because the sanity check on
finally block.
The resultset will only exist if and only if you already have a statement,
and "A ResultSet object is automatically closed when the Statement object
that generated it is closed, re-executed, or used to retrieve the next
result from a sequence of multiple results. ", thats why it is in a more
restrictive scope w/out close()
http://java.sun.com/j2se/1.4.2/docs/api/java/sql/ResultSet.html
On the race condition:
javadoc on Connection.close() states "Calling the method close on a
Connection object that is already closed is a no-op. ",
see: http://java.sun.com/javase/6/docs/api/java/sql/Connection.html#close()
This race condition should be threated as connection pool´s bug, IMHO.
Flavio Crispim
PS: English isn´t my mother language...
Alan Chaney <a...@compulsivecreative.com> gravou em 19/01/2009 21:24:43:
> Hi Stefan
>
> I went and read the comments more carefully and it seems to me that the
> proposed solution is an attempt to avoid a race condition between
> issuing the 'close' in
> one thread and then it being closed again whilst its being used in
> another thread.
>
> If the problem is closing it twice then I can't see why its not just
> closed in the 'finally' block, once for each thread.
>
> If it is a concurrency problem then I suspect that the proposed solution
> in the docs isn't the right one anyway. I'd suspect that the problem is
> more to do with threads not seeing each others memory state properly.
> Isn't this is a case of a variable (in this case 'conn') actually being
> a reference to an object which is shared between threads?
> Because of the JVM memory model it is possible for two threads not to be
> properly synchronized - see Doug Lea et.al and 'happens before' .
> Personally, I feel that the correct solution is to synchronize access
> to the connection object when it is retrieved and closed.
>
> I have to go out now and I don't have any more time to consider this
> today, but I'd be interested to hear other people's comments on this
topic.
>
> Regards
>
> Alan Chaney
>
>
> Stefan Riegel wrote:
> > Thanks Alan, just to make the thing really clear. You propose code
> > like this:
> >
> > public void execute() {
> > Connection conn = null;
> > Statement stmt = null;
> > ResultSet rs = null;
> > Context envContext = null;
> > try {
> > Context initContext = new InitialContext();
> > envContext = (Context)
initContext.lookup("java:/comp/env");
> > DataSource ds = (DataSource)
envContext.lookup("jdbc/swex");
> > conn = ds.getConnection();
> > stmt = conn.createStatement();
> > rs = stmt.executeQuery("some sql");
> > // iterate through the result set ...
> > } catch (SQLException e) {
> > e.printStackTrace();
> > } catch (NamingException e) {
> > e.printStackTrace();
> > } finally {
> > if (rs != null) {
> > try {
> > rs.close();
> > } catch (SQLException e) {
> > e.printStackTrace();
> > }
> > }
> > if (stmt != null) {
> > try {
> > stmt.close();
> > } catch (SQLException e) {
> > e.printStackTrace();
> > }
> > }
> > if (conn != null) {
> > try {
> > conn.close();
> > } catch (SQLException e) {
> > e.printStackTrace();
> > }
> > }
> > if (envContext != null) {
> > try {
> > envContext.close();
> > } catch (NamingException e) {
> > e.printStackTrace();
> > }
> > }
> > }
> >
> >
> > For me this looks fine but I'm still confused, why they complicated
> > things in the example of "properly written code"
> >
http://tomcat.apache.org/tomcat-6.0-doc/jndi-datasource-examples-howto.html?
> >
> >
> Hmm... frankly the code in the docs you refer to above seems odd to
> me... why repeat in the 'finally' something you've done in the 'try'?
>
> I can see that setting the variables to null would ensure that all
> references were released and the objects were made candidates for garbage
> collection but that could be done in the finally block anyway at some
> convenient point.
>
> Obviously in your code you might not want to obtain and release the
> context for every JDBC operation - that would probably be done in some
> kind of start-up/shutdown code for your app, and of course your
> exception handling may need some more work depending upon the way you
> want to present the error
> to your users, but I assumed that the issue you are concerned with is
> preventing resource leaks.
>
> Any comments anyone else?
>
> Regards
>
> Alan
>
>
>
> > Do I miss some important point here?
> >
> > Stefan
> >
> >
> > Alan Chaney schrieb:
> >> Hi Stefan
> >>
> >> You don't need to repeat the stmt.close();conn.close() etc in the
> >> 'try' body. The 'finally' by definition is ALWAYS called and that is
> >> where you should do the tidy up...
> >>
> >> Alan Chaney
> >>
> >>
> >>
> >> Stefan Riegel wrote:
> >>> I guess I understood the point with the "Random Connection Closed
> >>> Exceptions" Problem.
> >>>
> >>> See at the end of
> >>> http://tomcat.apache.org/tomcat-6.0-doc/jndi-datasource-
> examples-howto.html
> >>>
> >>>
> >>> As I understand, only the connection itself must be protected this
> >>> way. The statement and ResultSet must not. Is the following,
> >>> simplified code also correct?
> >>>
> >>> Connection conn = null;
> >>> Statement stmt = null;
> >>> ResultSet rs = null;
> >>> try {
> >>> conn = ... get connection from connection pool ...
> >>> stmt = conn.createStatement("select ...");
> >>> rs = stmt.executeQuery();
> >>> ... iterate through the result set ...
> >>
> >> // SUPERFLUsOUS
> >>> rs.close();
> >>> stmt.close();
> >>> conn.close(); // Return to connection pool
> >>> conn = null; // Make sure we don't close it twice
> >> //SUPERFLUOUS
> >>
> >>
> >>> } catch (SQLException e) {
> >>> ... deal with errors ...
> >>> } finally {
> >>> try {
> >>> rs.close();
> >>> } catch (SQLException e) {
> >>> // deal with errors
> >>> }
> >>> try {
> >>> stmt.close();
> >>> } catch (SQLException e) {
> >>> // deal with errors
> >>> }
> >>> if (conn != null) {
> >>> try {
> >>> conn.close();
> >>> } catch (SQLException e) {
> >>> // deal with errors
> >>> }
> >>> conn = null;
> >>> }
> >>> }
> >>>
> >>> Thanks.
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> >> For additional commands, e-mail: users-h...@tomcat.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
