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: [dbcp] Connection.close() implementation issues [was: Jumping in...]
Date Wed, 08 Mar 2006 14:09:25 GMT
On 3/8/06, Sandy McArthur <sandymac@apache.org> wrote:
> Sorry, ment to send this to commons-dev, not commons-user.
> /me needs sleep asap.
>
> ---------- Forwarded message ----------
> From: Sandy McArthur <sandymac@gmail.com>
> Date: Mar 8, 2006 2:00 AM
> Subject: [dbcp] Connection.close() implementation issues [was: Jumping in...]
> To: Jakarta Commons Users List <commons-user@jakarta.apache.org>
>
>
> On 3/7/06, Phil Steitz <phil.steitz@gmail.com> wrote:
> > There is a growing backlog of bugs open against [dbcp].  Unless other
> > committers object, I am going to jump in and start committing patches
> > and develop a maintenance release plan.  I plan to start with
> >
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=33591
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=38073
> >
> > I would appreciate any feedback that community members have on
> > prioritization of open tickets.  Also, as always, tickets with patches
> > and test cases are likely to get addressed quicker and comments /
> > testing / feedback are always welcome.
> >
> > Any help from other commons committers, even if just in the form of
> > screams, will be most appreciated.
>
> I've looked into some, as you can tell from my retracted bugzilla
> comment and I think I've found a large source of the problems.
>
> The method Connection.close() interface states: "Calling the method
> close on a Connection object that is already closed is a no-op."

Unless I am misunderstanding the use case in the ticket, it seems to
me that resolvoing it should lead us to qualify the above statement,
not "make it true".  We should search the archives for discussion on
this, as I recall there was a discussion of the "no op" issue.  IIUC,
the problem in the ticket has to do with the corner case when the
"already closed" state is entered without the knowledge of the pool.

One other thing to keep in mind is that we should be aiming for a
"dot-level" release (need to get a plan in place), so we are going to
be constrained by backward compatibility and we have to be careful
about changing behaviors that users may be counting on.  Therefore, we
should, wherever possible, apply the principle of least change when
fixing bugs.  Of course, that does not mean that we ignore significant
issues or break contracts.  Could be we what you are describing below
is appropriate and necessary.

Can you explain why the simple fixes in the ticket are not sufficient
to close the issue?

Thanks for looking into this.

Phil

>
> This is violated in a number of places:
> * PoolableConnection.java:77: throws an exception when close is called
> and it's already closed. It should be replaced with a line similar to:
> try {
>   if (!hasBeenReturnedToPool) {
>     hasBeenReturnedToPool = true;
>     _pool.invalidateObject(this);
>   }
> } catch (Exception e) {
>    // ignored
> }
>
> * ConnectionImpl.java:115: calls assertOpen() which throws an
> SQLException when it isn't open. This line should be removed.
>
> * PoolingDataSource.java:179: the call to checkOpen() is like the call
> to assertOpen() mentioned above and should be removed.
>
> * PoolingDriver:258: another call to checkOpen(), same solution as before.
>
> * TesterConnection.java:67: another call to checkOpen(), same solution
> as before.
>
> Someone should also audit:
> * DelegatingConnection.java:184: make sure the call to passivate() is
> fine being called more than once.
>
> --
> Sandy McArthur
>
> "He who dares not offend cannot be honest."
> - Thomas Paine
>
>
> --
> Sandy McArthur
>
> "He who dares not offend cannot be honest."
> - Thomas Paine
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>

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


Mime
View raw message