commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Phil Steitz" <phil.ste...@gmail.com>
Subject Re: "Already closed." exception in org.apache.commons.dbcp.PoolableConnection
Date Thu, 22 Nov 2007 03:54:53 GMT
On 11/21/07, Henri Yandell <flamefew@gmail.com> wrote:
> Especially as JDBC says:
>
> "Calling the method close on a Connection  object that is already
> closed is a no-op."
>
> I think this came up before though, so I suspect there's a good reason.
>
> Hen
>

There is a long history on this.  While I can't speak for the original
developers, one reason is that to hold onto a connection and close it
twice violates the pool contract.  Closing a connection from the pool
means "return the connection to the pool."  If DBCP did not guard
against this, returning a connection twice could result in two
references to the same connection being in the idle object pool at the
same time - obviously a bad situation.

As a result of the various bugs open against this (cf, e.g.
http://issues.apache.org/jira/browse/DBCP-233) and our (at least
Dain's and my) agreement that DBCP should be able to handle this, the
DBCP connection wrappers have been modified to support "multiple close
is no-op" behavior.  This will be released in DBCP 1.3.

I still think its a bad practice to close a pooled connection multiple
times, though.  References to pooled resources should never be used
once the resources have been returned to the pool. If this is
happening in your code, you should find out why and eliminate it,
since this may indicate other problems.

Phil

> On Nov 19, 2007 4:28 PM,  <Jeffrey.Constantin@cox.com> wrote:
> > To Whom It May Concern:
> >
> > Why is it necessary to throw a new SQLException("Already closed") when
> > the purpose of the close() method is to close the connection. Seems to
> > me this Exception is not necessary.
> >
> > In my code (Tomcat/DBCP/iBATIS) apparently iBATIS is trying to close an
> > already closed connection( without checking ) and this causes the
> > close() method to throw the exception.
> >
> > Any design thoughts are greatly appreciated.
> >
> > Thanks,
> >
> > Jeff
> >
> >     /**
> >      * Returns me to my pool.
> >      */
> >      public synchronized void close() throws SQLException {
> >         boolean isClosed = false;
> >         try {
> >             isClosed = isClosed();
> >         } catch (SQLException e) {
> >             try {
> >                 _pool.invalidateObject(this); // XXX should be guarded
> > to happen at most once
> >             } catch (Exception ie) {
> >                 // DO NOTHING the original exception will be rethrown
> >             }
> >             throw new SQLNestedException("Cannot close connection
> > (isClosed check failed)", e);
> >         }
> >         if (isClosed) {
> >             try {
> >                 _pool.invalidateObject(this); // XXX should be guarded
> > to happen at most once
> >             } catch (Exception ie) {
> >                 // DO NOTHING, "Already closed" exception thrown below
> >             }
> >             throw new SQLException("Already closed.");
> >         } else {
> >             try {
> >                 _pool.returnObject(this); // XXX should be guarded to
> > happen at most once
> >             } catch(SQLException e) {
> >                 throw e;
> >             } catch(RuntimeException e) {
> >                 throw e;
> >             } catch(Exception e) {
> >                 throw new SQLNestedException("Cannot close connection
> > (return to pool failed)", e);
> >             }
> >         }
> >     }
> >
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message