1.3 release problems

classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

1.3 release problems

Phil Steitz
I am running into some problems preparing for dbcp-1.3.  I would
appreciate comments / patches on any of the issues below.

1. Findbugs is showing some real (inconsistent synch) and not so
real (e.g. serialization issues on classes that IMO should not be
serializable, but we can't fix until 2.0).  The full report is here:
http://commons.apache.org/dbcp/findbugs.html
I would appreciate suggestions/patches/commits for what to fix and how.

2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
as the code stands today).  So we need to create two jar artifacts.
 The question is which one gets the 1.3 name, what is the other
named and how do we package the distros?

3. I assume it is OK at this point to drop the nojdbc3 Ant target
and compiler flags for JDBC 2.

TIA

Phil

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: 1.3 release problems

Dennis Lundberg-2
Phil Steitz wrote:

> I am running into some problems preparing for dbcp-1.3.  I would
> appreciate comments / patches on any of the issues below.
>
> 1. Findbugs is showing some real (inconsistent synch) and not so
> real (e.g. serialization issues on classes that IMO should not be
> serializable, but we can't fix until 2.0).  The full report is here:
> http://commons.apache.org/dbcp/findbugs.html
> I would appreciate suggestions/patches/commits for what to fix and how.
>
> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
> as the code stands today).  So we need to create two jar artifacts.
>  The question is which one gets the 1.3 name, what is the other
> named and how do we package the distros?

I see two possible naming schemes for this, both of them using
classifiers in the same way that Maven does.

1. commons-pool-1.3-jdk14.jar and commons-pool-1.3-jdk16.jar

2. commons-pool-1.3-jdbc3.jar and commons-pool-1.3-jdbc4.jar

Both of these schemes have a classifier on *both* artifacts. If you want
to eliminate one of the classifiers I'd propose to skip the classifier
on the JDBC 3 artifact, since that is what is currently out there in the
1.2.x versions of commons-dbcp. Then it would be

3. commons-pool-1.3.jar and commons-pool-1.3-jdk16.jar

4. commons-pool-1.3.jar and commons-pool-1.3-jdbc4.jar


>
> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
> and compiler flags for JDBC 2.
>
> TIA
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


--
Dennis Lundberg

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: 1.3 release problems

sebb-2-2
In reply to this post by Phil Steitz
On 22/11/2009, Phil Steitz <[hidden email]> wrote:
> I am running into some problems preparing for dbcp-1.3.  I would
>  appreciate comments / patches on any of the issues below.
>
>  1. Findbugs is showing some real (inconsistent synch) and not so
>  real (e.g. serialization issues on classes that IMO should not be
>  serializable, but we can't fix until 2.0).  The full report is here:
>  http://commons.apache.org/dbcp/findbugs.html
>  I would appreciate suggestions/patches/commits for what to fix and how.

org.apache.commons.dbcp.AbandonedTrace$AbandonedObjectException.format
- not a problem, as the code is synch. on format, just disable the report

org.apache.commons.dbcp.PoolableConnectionFactory._connFactory,_pool,_validationQuery
=> just make these volatile.

org.apache.commons.dbcp.PoolingConnection.createKey(String, byte)
might ignore java.lang.Exception (lines218, 229, 240 and 251)
No idea

PoolingConnection$PStmtKey.PoolingConnection$PStmtKey._resultSetType
could be null and is guaranteed to be dereferenced in
org.apache.commons.dbcp.PoolingConnection.makeObject(Object)
This looks like a bug; just check for null in the second condition?

Class org.apache.commons.dbcp.cpdsadapter.DriverAdapterCPDS defines
non-transient non-serializable instance field logWriter
Just make the logWriter transient.

_pool synch: add synch or make volatile.
<aside>
Seems to me a lot of these synch. problems would be avoided if the
variables did not have set() methods - why are there set() methods for
fields that are provided in the constructors? What is the use case for
this?
</aside>

It would be helpful to know which classes are intended to be
thread-safe, as it's not clear whether the potential synch. problems
are likely to occur in normal usage or not.

For example the class SharedPoolDataSource: the field "pool" is
sometimes synch., and sometimes not, but the fields maxActive,
maxWait, maxIdle are not synch. at all.

The use of synchronization seems rather haphazard to me.

>  2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>  and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>  as the code stands today).  So we need to create two jar artifacts.

How difficult would it be to support both in the same jar?

>   The question is which one gets the 1.3 name, what is the other
>  named and how do we package the distros?
>
>  3. I assume it is OK at this point to drop the nojdbc3 Ant target
>  and compiler flags for JDBC 2.
>
>  TIA
>
>  Phil
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: [hidden email]
>  For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: 1.3 release problems

Phil Steitz
sebb wrote:

> On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>> I am running into some problems preparing for dbcp-1.3.  I would
>>  appreciate comments / patches on any of the issues below.
>>
>>  1. Findbugs is showing some real (inconsistent synch) and not so
>>  real (e.g. serialization issues on classes that IMO should not be
>>  serializable, but we can't fix until 2.0).  The full report is here:
>>  http://commons.apache.org/dbcp/findbugs.html
>>  I would appreciate suggestions/patches/commits for what to fix and how.
>
> org.apache.commons.dbcp.AbandonedTrace$AbandonedObjectException.format
> - not a problem, as the code is synch. on format, just disable the report

+1
>
> org.apache.commons.dbcp.PoolableConnectionFactory._connFactory,_pool,_validationQuery
> => just make these volatile.

+1 - all we can do without breaking compat
>
> org.apache.commons.dbcp.PoolingConnection.createKey(String, byte)
> might ignore java.lang.Exception (lines218, 229, 240 and 251)
> No idea

This is silly - exceptions potentially thrown by getCatalog are
(intentionally) swallowed.
>
> PoolingConnection$PStmtKey.PoolingConnection$PStmtKey._resultSetType
> could be null and is guaranteed to be dereferenced in
> org.apache.commons.dbcp.PoolingConnection.makeObject(Object)
> This looks like a bug; just check for null in the second condition?

Should never happen, but will refactor to explicitly avoid.
>
> Class org.apache.commons.dbcp.cpdsadapter.DriverAdapterCPDS defines
> non-transient non-serializable instance field logWriter
> Just make the logWriter transient.

+1
>
> _pool synch: add synch or make volatile.

I guess make volatile is safest.
> <aside>
> Seems to me a lot of these synch. problems would be avoided if the
> variables did not have set() methods - why are there set() methods for
> fields that are provided in the constructors? What is the use case for
> this?
> </aside>

Agree strongly with comment.  BasicDataSource is crippled by this.
It is effectively immutable once getConnection has been called, but
the public setters and protected fields make it impossible to fix
without breaking compatibility.  See DBCP-300 for example of how
this causes needless performance problems. For Tomcat, I have been
thinking about providing an alternative JNDI factory that returns a
PoolingDataSource instead.

>
> It would be helpful to know which classes are intended to be
> thread-safe, as it's not clear whether the potential synch. problems
> are likely to occur in normal usage or not.
>
> For example the class SharedPoolDataSource: the field "pool" is
> sometimes synch., and sometimes not, but the fields maxActive,
> maxWait, maxIdle are not synch. at all.

Here again, all of these should be immutable properties set by the
constructor.
>
> The use of synchronization seems rather haphazard to me.

harsh but true ;)  Comment above really covers it - the needlessly
sloppy synch is in most cases due to overly mutable - and sometimes
directly exposed - properties and no concern for synch issues that
are not likely to occur in normal use.  I am +1 for fixing anything
that we can pre-2.0 subject to compat and performance constraints.
>
>>  2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>>  and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>>  as the code stands today).  So we need to create two jar artifacts.
>
> How difficult would it be to support both in the same jar?

I would like to do that if we could do it safely.  I have not been
able to get the 1.6-compiled jar to successfully run the tests
compiled against 1.5.

The failure that I get when using Ant to compile and execute the
tests (commenting out the 1.6-stuff in the test classes) using a
1.6-built jar is strange:

[junit] Exception in thread "Thread-16"
java.lang.NoClassDefFoundError: java/sql/SQLClientInfoException
    [junit] at
org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:592)
    [junit] at
org.apache.commons.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1537)
    [junit] at
org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1526)
    [junit] at
org.apache.commons.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1374)
    [junit] at
org.apache.commons.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1038)
    [junit] at
org.apache.commons.dbcp.TestBasicDataSource.getConnection(TestBasicDataSource.java:44)
    [junit] at
org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84)
    [junit] at
org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595)
    [junit] at java.lang.Thread.run(Thread.java:613)

Strange as the line number in PCF.makeObject and missing class makes
no sense.


Thanks, Sebb!

>
>>   The question is which one gets the 1.3 name, what is the other
>>  named and how do we package the distros?
>>
>>  3. I assume it is OK at this point to drop the nojdbc3 Ant target
>>  and compiler flags for JDBC 2.
>>
>>  TIA
>>
>>  Phil
>>
>>  ---------------------------------------------------------------------
>>  To unsubscribe, e-mail: [hidden email]
>>  For additional commands, e-mail: [hidden email]
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: 1.3 release problems

Phil Steitz
In reply to this post by Dennis Lundberg-2
Dennis Lundberg wrote:

> Phil Steitz wrote:
>> I am running into some problems preparing for dbcp-1.3.  I would
>> appreciate comments / patches on any of the issues below.
>>
>> 1. Findbugs is showing some real (inconsistent synch) and not so
>> real (e.g. serialization issues on classes that IMO should not be
>> serializable, but we can't fix until 2.0).  The full report is here:
>> http://commons.apache.org/dbcp/findbugs.html
>> I would appreciate suggestions/patches/commits for what to fix and how.
>>
>> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>> as the code stands today).  So we need to create two jar artifacts.
>>  The question is which one gets the 1.3 name, what is the other
>> named and how do we package the distros?
>
> I see two possible naming schemes for this, both of them using
> classifiers in the same way that Maven does.
>
> 1. commons-pool-1.3-jdk14.jar and commons-pool-1.3-jdk16.jar
>
> 2. commons-pool-1.3-jdbc3.jar and commons-pool-1.3-jdbc4.jar
>
> Both of these schemes have a classifier on *both* artifacts. If you want
> to eliminate one of the classifiers I'd propose to skip the classifier
> on the JDBC 3 artifact, since that is what is currently out there in the
> 1.2.x versions of commons-dbcp. Then it would be
>
> 3. commons-pool-1.3.jar and commons-pool-1.3-jdk16.jar
>
> 4. commons-pool-1.3.jar and commons-pool-1.3-jdbc4.jar

Thanks, Dennis!

Phil

>
>
>> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
>> and compiler flags for JDBC 2.
>>
>> TIA
>>
>> Phil
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: 1.3 release problems

Gary Gregory
In reply to this post by Phil Steitz
Does dropping support for Java 1.4 simply the issue?

Gary

> -----Original Message-----
> From: Phil Steitz [mailto:[hidden email]]
> Sent: Sunday, November 22, 2009 08:43
> To: Commons Developers List
> Subject: [dbcp] 1.3 release problems
>
> I am running into some problems preparing for dbcp-1.3.  I would
> appreciate comments / patches on any of the issues below.
>
> 1. Findbugs is showing some real (inconsistent synch) and not so
> real (e.g. serialization issues on classes that IMO should not be
> serializable, but we can't fix until 2.0).  The full report is here:
> http://commons.apache.org/dbcp/findbugs.html
> I would appreciate suggestions/patches/commits for what to fix and how.
>
> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
> as the code stands today).  So we need to create two jar artifacts.
>  The question is which one gets the 1.3 name, what is the other
> named and how do we package the distros?
>
> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
> and compiler flags for JDBC 2.
>
> TIA
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: 1.3 release problems

sebb-2-2
In reply to this post by Phil Steitz
On 22/11/2009, Phil Steitz <[hidden email]> wrote:

> sebb wrote:
>  > On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>  >> I am running into some problems preparing for dbcp-1.3.  I would
>  >>  appreciate comments / patches on any of the issues below.
>  >>
>  >>  1. Findbugs is showing some real (inconsistent synch) and not so
>  >>  real (e.g. serialization issues on classes that IMO should not be
>  >>  serializable, but we can't fix until 2.0).  The full report is here:
>  >>  http://commons.apache.org/dbcp/findbugs.html
>  >>  I would appreciate suggestions/patches/commits for what to fix and how.
>  >
>  > org.apache.commons.dbcp.AbandonedTrace$AbandonedObjectException.format
>  > - not a problem, as the code is synch. on format, just disable the report
>
>
> +1
>
> >
>  > org.apache.commons.dbcp.PoolableConnectionFactory._connFactory,_pool,_validationQuery
>  > => just make these volatile.
>
>
> +1 - all we can do without breaking compat
>
> >
>  > org.apache.commons.dbcp.PoolingConnection.createKey(String, byte)
>  > might ignore java.lang.Exception (lines218, 229, 240 and 251)
>  > No idea
>
>
> This is silly - exceptions potentially thrown by getCatalog are
>  (intentionally) swallowed.

However the methods should only catch SQLException, not Exception.

_catalog should probably have been final and private. Or could
probably be dropped altogether, as it does not seem to be necessary.

> >
>  > PoolingConnection$PStmtKey.PoolingConnection$PStmtKey._resultSetType
>  > could be null and is guaranteed to be dereferenced in
>  > org.apache.commons.dbcp.PoolingConnection.makeObject(Object)
>  > This looks like a bug; just check for null in the second condition?
>
>
> Should never happen, but will refactor to explicitly avoid.
>
> >
>  > Class org.apache.commons.dbcp.cpdsadapter.DriverAdapterCPDS defines
>  > non-transient non-serializable instance field logWriter
>  > Just make the logWriter transient.
>
>
> +1
>
> >
>  > _pool synch: add synch or make volatile.
>
>
> I guess make volatile is safest.
>
> > <aside>
>  > Seems to me a lot of these synch. problems would be avoided if the
>  > variables did not have set() methods - why are there set() methods for
>  > fields that are provided in the constructors? What is the use case for
>  > this?
>  > </aside>
>
>
> Agree strongly with comment.  BasicDataSource is crippled by this.
>  It is effectively immutable once getConnection has been called, but
>  the public setters and protected fields make it impossible to fix
>  without breaking compatibility.  See DBCP-300 for example of how
>  this causes needless performance problems. For Tomcat, I have been
>  thinking about providing an alternative JNDI factory that returns a
>  PoolingDataSource instead.
>
>
>  >
>  > It would be helpful to know which classes are intended to be
>  > thread-safe, as it's not clear whether the potential synch. problems
>  > are likely to occur in normal usage or not.
>  >
>  > For example the class SharedPoolDataSource: the field "pool" is
>  > sometimes synch., and sometimes not, but the fields maxActive,
>  > maxWait, maxIdle are not synch. at all.
>
>
> Here again, all of these should be immutable properties set by the
>  constructor.
>
> >
>  > The use of synchronization seems rather haphazard to me.
>
>
> harsh but true ;)  Comment above really covers it - the needlessly
>  sloppy synch is in most cases due to overly mutable - and sometimes
>  directly exposed - properties and no concern for synch issues that
>  are not likely to occur in normal use.  I am +1 for fixing anything
>  that we can pre-2.0 subject to compat and performance constraints.
>
> >
>  >>  2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>  >>  and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>  >>  as the code stands today).  So we need to create two jar artifacts.
>  >
>  > How difficult would it be to support both in the same jar?
>
>
> I would like to do that if we could do it safely.  I have not been
>  able to get the 1.6-compiled jar to successfully run the tests
>  compiled against 1.5.
>
>  The failure that I get when using Ant to compile and execute the
>  tests (commenting out the 1.6-stuff in the test classes) using a
>  1.6-built jar is strange:
>
>  [junit] Exception in thread "Thread-16"
>  java.lang.NoClassDefFoundError: java/sql/SQLClientInfoException
>     [junit]     at
>  org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:592)
>     [junit]     at
>  org.apache.commons.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1537)
>     [junit]     at
>  org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1526)
>     [junit]     at
>  org.apache.commons.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1374)
>     [junit]     at
>  org.apache.commons.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1038)
>     [junit]     at
>  org.apache.commons.dbcp.TestBasicDataSource.getConnection(TestBasicDataSource.java:44)
>     [junit]     at
>  org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84)
>     [junit]     at
>  org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595)
>     [junit]     at java.lang.Thread.run(Thread.java:613)
>
>  Strange as the line number in PCF.makeObject and missing class makes
>  no sense.
>
>
>  Thanks, Sebb!
>
> >
>  >>   The question is which one gets the 1.3 name, what is the other
>  >>  named and how do we package the distros?
>  >>
>  >>  3. I assume it is OK at this point to drop the nojdbc3 Ant target
>  >>  and compiler flags for JDBC 2.
>  >>
>  >>  TIA
>  >>
>  >>  Phil
>  >>
>  >>  ---------------------------------------------------------------------
>  >>  To unsubscribe, e-mail: [hidden email]
>  >>  For additional commands, e-mail: [hidden email]
>  >>
>  >>
>  >
>  > ---------------------------------------------------------------------
>  > To unsubscribe, e-mail: [hidden email]
>  > For additional commands, e-mail: [hidden email]
>  >
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: [hidden email]
>  For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: 1.3 release problems

Phil Steitz
In reply to this post by Gary Gregory
Gary Gregory wrote:
> Does dropping support for Java 1.4 simply the issue?

No. Would have to drop 1.5 too.

Phil

>
> Gary
>
>> -----Original Message-----
>> From: Phil Steitz [mailto:[hidden email]]
>> Sent: Sunday, November 22, 2009 08:43
>> To: Commons Developers List
>> Subject: [dbcp] 1.3 release problems
>>
>> I am running into some problems preparing for dbcp-1.3.  I would
>> appreciate comments / patches on any of the issues below.
>>
>> 1. Findbugs is showing some real (inconsistent synch) and not so
>> real (e.g. serialization issues on classes that IMO should not be
>> serializable, but we can't fix until 2.0).  The full report is here:
>> http://commons.apache.org/dbcp/findbugs.html
>> I would appreciate suggestions/patches/commits for what to fix and how.
>>
>> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>> as the code stands today).  So we need to create two jar artifacts.
>>  The question is which one gets the 1.3 name, what is the other
>> named and how do we package the distros?
>>
>> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
>> and compiler flags for JDBC 2.
>>
>> TIA
>>
>> Phil
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: 1.3 release problems

Grzegorz S?owikowski
In reply to this post by Phil Steitz
Hi

Phil Steitz wrote:

> I am running into some problems preparing for dbcp-1.3.  I would
> appreciate comments / patches on any of the issues below.
>
> 1. Findbugs is showing some real (inconsistent synch) and not so
> real (e.g. serialization issues on classes that IMO should not be
> serializable, but we can't fix until 2.0).  The full report is here:
> http://commons.apache.org/dbcp/findbugs.html
> I would appreciate suggestions/patches/commits for what to fix and how.
>
> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
> as the code stands today).  So we need to create two jar artifacts.
>  The question is which one gets the 1.3 name, what is the other
> named and how do we package the distros?
>
>  
How come the old JDBC3 database drivers (for example I'm using Oracle
ojdbc14.jar)
are useable in Java 1.6? They are. I don't know how, maybe there is
something
inside running JVM. So why commons-dbcp compiled with Java 1.4 or 1.5
will not
be compatible with Java 1.6 in runtime?

> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
> and compiler flags for JDBC 2.
>
> TIA
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
>  

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: [dbcp] 1.3 release problems

Gary Gregory
In reply to this post by Phil Steitz
Our server and tools are all on Java 6, so for us, moving to 6 is quite fine.

We strongly encourage our users to move away from 1.4.2 ASAP.

Gary

> -----Original Message-----
> From: Phil Steitz [mailto:[hidden email]]
> Sent: Sunday, November 22, 2009 14:03
> To: Commons Developers List
> Subject: Re: [dbcp] 1.3 release problems
>
> Gary Gregory wrote:
> > Does dropping support for Java 1.4 simply the issue?
>
> No. Would have to drop 1.5 too.
>
> Phil
> >
> > Gary
> >
> >> -----Original Message-----
> >> From: Phil Steitz [mailto:[hidden email]]
> >> Sent: Sunday, November 22, 2009 08:43
> >> To: Commons Developers List
> >> Subject: [dbcp] 1.3 release problems
> >>
> >> I am running into some problems preparing for dbcp-1.3.  I would
> >> appreciate comments / patches on any of the issues below.
> >>
> >> 1. Findbugs is showing some real (inconsistent synch) and not so
> >> real (e.g. serialization issues on classes that IMO should not be
> >> serializable, but we can't fix until 2.0).  The full report is here:
> >> http://commons.apache.org/dbcp/findbugs.html
> >> I would appreciate suggestions/patches/commits for what to fix and
> how.
> >>
> >> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
> >> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
> >> as the code stands today).  So we need to create two jar artifacts.
> >>  The question is which one gets the 1.3 name, what is the other
> >> named and how do we package the distros?
> >>
> >> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
> >> and compiler flags for JDBC 2.
> >>
> >> TIA
> >>
> >> Phil
> >>
> >> --------------------------------------------------------------------
> -
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

Phil Steitz
In reply to this post by Grzegorz S?owikowski
Grzegorz S?owikowski wrote:

> Hi
>
> Phil Steitz wrote:
>> I am running into some problems preparing for dbcp-1.3.  I would
>> appreciate comments / patches on any of the issues below.
>>
>> 1. Findbugs is showing some real (inconsistent synch) and not so
>> real (e.g. serialization issues on classes that IMO should not be
>> serializable, but we can't fix until 2.0).  The full report is here:
>> http://commons.apache.org/dbcp/findbugs.html
>> I would appreciate suggestions/patches/commits for what to fix and how.
>>
>> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>> as the code stands today).  So we need to create two jar artifacts.
>>  The question is which one gets the 1.3 name, what is the other
>> named and how do we package the distros?
>>
>>  
> How come the old JDBC3 database drivers (for example I'm using Oracle
> ojdbc14.jar)
> are useable in Java 1.6? They are. I don't know how, maybe there is
> something
> inside running JVM. So why commons-dbcp compiled with Java 1.4 or 1.5
> will not
> be compatible with Java 1.6 in runtime?

The problem is not that (other than problems compiling from source,
which the Ant build has fixed).  The problem is that if we want to
produce just one jar, we need to compile it using JDK 1.6 and that
jar does not work (at least for me) when used with older JDKs.

Phil

>
>> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
>> and compiler flags for JDBC 2.
>>
>> TIA
>>
>> Phil
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>>  
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

Phil Steitz
In reply to this post by sebb-2-2
sebb wrote:

> On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>> sebb wrote:
>>  > On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>>  >> I am running into some problems preparing for dbcp-1.3.  I would
>>  >>  appreciate comments / patches on any of the issues below.
>>  >>
>>  >>  1. Findbugs is showing some real (inconsistent synch) and not so
>>  >>  real (e.g. serialization issues on classes that IMO should not be
>>  >>  serializable, but we can't fix until 2.0).  The full report is here:
>>  >>  http://commons.apache.org/dbcp/findbugs.html
>>  >>  I would appreciate suggestions/patches/commits for what to fix and how.
>>  >
>>  > org.apache.commons.dbcp.AbandonedTrace$AbandonedObjectException.format
>>  > - not a problem, as the code is synch. on format, just disable the report
>>
>>
>> +1
>>
>>  > org.apache.commons.dbcp.PoolableConnectionFactory._connFactory,_pool,_validationQuery
>>  > => just make these volatile.
>>
>>
>> +1 - all we can do without breaking compat
>>
>>  > org.apache.commons.dbcp.PoolingConnection.createKey(String, byte)
>>  > might ignore java.lang.Exception (lines218, 229, 240 and 251)
>>  > No idea
>>
>>
>> This is silly - exceptions potentially thrown by getCatalog are
>>  (intentionally) swallowed.
>
> However the methods should only catch SQLException, not Exception.

+1 - and that we can fix.
>
> _catalog should probably have been final and private. Or could
> probably be dropped altogether, as it does not seem to be necessary.

I think the field is necessary - at least there was a BZ ticket that
caused it to be added ([Bug 27246] - PreparedStatement cache should
be different depending on the Catalog).  Agree it and other key
fields should be final.  Unfortunately, they are all protected now,
so to fix is to break.

>
>>  > PoolingConnection$PStmtKey.PoolingConnection$PStmtKey._resultSetType
>>  > could be null and is guaranteed to be dereferenced in
>>  > org.apache.commons.dbcp.PoolingConnection.makeObject(Object)
>>  > This looks like a bug; just check for null in the second condition?
>>
>>
>> Should never happen, but will refactor to explicitly avoid.
>>
>>  > Class org.apache.commons.dbcp.cpdsadapter.DriverAdapterCPDS defines
>>  > non-transient non-serializable instance field logWriter
>>  > Just make the logWriter transient.
>>
>>
>> +1
>>
>>  > _pool synch: add synch or make volatile.
>>
>>
>> I guess make volatile is safest.
>>
>>> <aside>
>>  > Seems to me a lot of these synch. problems would be avoided if the
>>  > variables did not have set() methods - why are there set() methods for
>>  > fields that are provided in the constructors? What is the use case for
>>  > this?
>>  > </aside>
>>
>>
>> Agree strongly with comment.  BasicDataSource is crippled by this.
>>  It is effectively immutable once getConnection has been called, but
>>  the public setters and protected fields make it impossible to fix
>>  without breaking compatibility.  See DBCP-300 for example of how
>>  this causes needless performance problems. For Tomcat, I have been
>>  thinking about providing an alternative JNDI factory that returns a
>>  PoolingDataSource instead.
>>
>>
>>  >
>>  > It would be helpful to know which classes are intended to be
>>  > thread-safe, as it's not clear whether the potential synch. problems
>>  > are likely to occur in normal usage or not.
>>  >
>>  > For example the class SharedPoolDataSource: the field "pool" is
>>  > sometimes synch., and sometimes not, but the fields maxActive,
>>  > maxWait, maxIdle are not synch. at all.
>>
>>
>> Here again, all of these should be immutable properties set by the
>>  constructor.
>>
>>  > The use of synchronization seems rather haphazard to me.
>>
>>
>> harsh but true ;)  Comment above really covers it - the needlessly
>>  sloppy synch is in most cases due to overly mutable - and sometimes
>>  directly exposed - properties and no concern for synch issues that
>>  are not likely to occur in normal use.  I am +1 for fixing anything
>>  that we can pre-2.0 subject to compat and performance constraints.
>>
>>  >>  2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>>  >>  and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>>  >>  as the code stands today).  So we need to create two jar artifacts.
>>  >
>>  > How difficult would it be to support both in the same jar?
>>
>>
>> I would like to do that if we could do it safely.  I have not been
>>  able to get the 1.6-compiled jar to successfully run the tests
>>  compiled against 1.5.
>>
>>  The failure that I get when using Ant to compile and execute the
>>  tests (commenting out the 1.6-stuff in the test classes) using a
>>  1.6-built jar is strange:
>>
>>  [junit] Exception in thread "Thread-16"
>>  java.lang.NoClassDefFoundError: java/sql/SQLClientInfoException
>>     [junit]     at
>>  org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:592)
>>     [junit]     at
>>  org.apache.commons.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1537)
>>     [junit]     at
>>  org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1526)
>>     [junit]     at
>>  org.apache.commons.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1374)
>>     [junit]     at
>>  org.apache.commons.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1038)
>>     [junit]     at
>>  org.apache.commons.dbcp.TestBasicDataSource.getConnection(TestBasicDataSource.java:44)
>>     [junit]     at
>>  org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84)
>>     [junit]     at
>>  org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595)
>>     [junit]     at java.lang.Thread.run(Thread.java:613)
>>
>>  Strange as the line number in PCF.makeObject and missing class makes
>>  no sense.
>>
>>
>>  Thanks, Sebb!
>>
>>  >>   The question is which one gets the 1.3 name, what is the other
>>  >>  named and how do we package the distros?
>>  >>
>>  >>  3. I assume it is OK at this point to drop the nojdbc3 Ant target
>>  >>  and compiler flags for JDBC 2.
>>  >>
>>  >>  TIA
>>  >>
>>  >>  Phil
>>  >>
>>  >>  ---------------------------------------------------------------------
>>  >>  To unsubscribe, e-mail: [hidden email]
>>  >>  For additional commands, e-mail: [hidden email]
>>  >>
>>  >>
>>  >
>>  > ---------------------------------------------------------------------
>>  > To unsubscribe, e-mail: [hidden email]
>>  > For additional commands, e-mail: [hidden email]
>>  >
>>
>>
>>  ---------------------------------------------------------------------
>>  To unsubscribe, e-mail: [hidden email]
>>  For additional commands, e-mail: [hidden email]
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

Henri Yandell
In reply to this post by Gary Gregory
Given that the Sun JDK's SSL implementation has the same problem as
other SSL implementations, and given that Sun don't support the free
1.4 and 1.5 versions (so no security fixes coming), I think we do a
disservice in even encouraging people to stay on 1.4 and 1.5 by
continuing to support them.

1.6 for all Commons components :)

Hen

On Sun, Nov 22, 2009 at 5:58 PM, Gary Gregory
<[hidden email]> wrote:

> Our server and tools are all on Java 6, so for us, moving to 6 is quite fine.
>
> We strongly encourage our users to move away from 1.4.2 ASAP.
>
> Gary
>
>> -----Original Message-----
>> From: Phil Steitz [mailto:[hidden email]]
>> Sent: Sunday, November 22, 2009 14:03
>> To: Commons Developers List
>> Subject: Re: [dbcp] 1.3 release problems
>>
>> Gary Gregory wrote:
>> > Does dropping support for Java 1.4 simply the issue?
>>
>> No. Would have to drop 1.5 too.
>>
>> Phil
>> >
>> > Gary
>> >
>> >> -----Original Message-----
>> >> From: Phil Steitz [mailto:[hidden email]]
>> >> Sent: Sunday, November 22, 2009 08:43
>> >> To: Commons Developers List
>> >> Subject: [dbcp] 1.3 release problems
>> >>
>> >> I am running into some problems preparing for dbcp-1.3.  I would
>> >> appreciate comments / patches on any of the issues below.
>> >>
>> >> 1. Findbugs is showing some real (inconsistent synch) and not so
>> >> real (e.g. serialization issues on classes that IMO should not be
>> >> serializable, but we can't fix until 2.0).  The full report is here:
>> >> http://commons.apache.org/dbcp/findbugs.html
>> >> I would appreciate suggestions/patches/commits for what to fix and
>> how.
>> >>
>> >> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>> >> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>> >> as the code stands today).  So we need to create two jar artifacts.
>> >>  The question is which one gets the 1.3 name, what is the other
>> >> named and how do we package the distros?
>> >>
>> >> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
>> >> and compiler flags for JDBC 2.
>> >>
>> >> TIA
>> >>
>> >> Phil
>> >>
>> >> --------------------------------------------------------------------
>> -
>> >> To unsubscribe, e-mail: [hidden email]
>> >> For additional commands, e-mail: [hidden email]
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: [hidden email]
>> > For additional commands, e-mail: [hidden email]
>> >
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

Jörg Schaible
In reply to this post by Phil Steitz
Phil Steitz wrote at Sonntag, 22. November 2009 17:43:

> I am running into some problems preparing for dbcp-1.3.  I would
> appreciate comments / patches on any of the issues below.
>
> 1. Findbugs is showing some real (inconsistent synch) and not so
> real (e.g. serialization issues on classes that IMO should not be
> serializable, but we can't fix until 2.0).  The full report is here:
> http://commons.apache.org/dbcp/findbugs.html
> I would appreciate suggestions/patches/commits for what to fix and how.
>
> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
> as the code stands today).  So we need to create two jar artifacts.
>  The question is which one gets the 1.3 name, what is the other
> named and how do we package the distros?
>
> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
> and compiler flags for JDBC 2.

Do you think it can be solved, if you compile anything with target 1.4 while
compiling the remaining stuff with target 1.6? The resulting jar will
contain a mixture of both class types. We do that since years for XStream,
see the different executions for the compiler plugin:
http://svn.xstream.codehaus.org/browse/xstream/trunk/xstream/pom.xml?r=HEAD

- Jörg


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: [dbcp] 1.3 release problems

Gary Gregory
In reply to this post by Henri Yandell
> -----Original Message-----
> From: Henri Yandell [mailto:[hidden email]]
> Sent: Sunday, November 22, 2009 21:17
> To: Commons Developers List
> Subject: Re: [dbcp] 1.3 release problems
>
> Given that the Sun JDK's SSL implementation has the same problem as
> other SSL implementations, and given that Sun don't support the free
> 1.4 and 1.5 versions (so no security fixes coming), I think we do a
> disservice in even encouraging people to stay on 1.4 and 1.5 by
> continuing to support them.
>
> 1.6 for all Commons components :)

+1!

Gary

>
> Hen
>
> On Sun, Nov 22, 2009 at 5:58 PM, Gary Gregory
> <[hidden email]> wrote:
> > Our server and tools are all on Java 6, so for us, moving to 6 is
> quite fine.
> >
> > We strongly encourage our users to move away from 1.4.2 ASAP.
> >
> > Gary
> >
> >> -----Original Message-----
> >> From: Phil Steitz [mailto:[hidden email]]
> >> Sent: Sunday, November 22, 2009 14:03
> >> To: Commons Developers List
> >> Subject: Re: [dbcp] 1.3 release problems
> >>
> >> Gary Gregory wrote:
> >> > Does dropping support for Java 1.4 simply the issue?
> >>
> >> No. Would have to drop 1.5 too.
> >>
> >> Phil
> >> >
> >> > Gary
> >> >
> >> >> -----Original Message-----
> >> >> From: Phil Steitz [mailto:[hidden email]]
> >> >> Sent: Sunday, November 22, 2009 08:43
> >> >> To: Commons Developers List
> >> >> Subject: [dbcp] 1.3 release problems
> >> >>
> >> >> I am running into some problems preparing for dbcp-1.3.  I would
> >> >> appreciate comments / patches on any of the issues below.
> >> >>
> >> >> 1. Findbugs is showing some real (inconsistent synch) and not so
> >> >> real (e.g. serialization issues on classes that IMO should not be
> >> >> serializable, but we can't fix until 2.0).  The full report is
> here:
> >> >> http://commons.apache.org/dbcp/findbugs.html
> >> >> I would appreciate suggestions/patches/commits for what to fix
> and
> >> how.
> >> >>
> >> >> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
> >> >> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least
> not
> >> >> as the code stands today).  So we need to create two jar
> artifacts.
> >> >>  The question is which one gets the 1.3 name, what is the other
> >> >> named and how do we package the distros?
> >> >>
> >> >> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
> >> >> and compiler flags for JDBC 2.
> >> >>
> >> >> TIA
> >> >>
> >> >> Phil
> >> >>
> >> >> -----------------------------------------------------------------
> ---
> >> -
> >> >> To unsubscribe, e-mail: [hidden email]
> >> >> For additional commands, e-mail: [hidden email]
> >> >
> >> >
> >> > ------------------------------------------------------------------
> ---
> >> > To unsubscribe, e-mail: [hidden email]
> >> > For additional commands, e-mail: [hidden email]
> >> >
> >>
> >>
> >> --------------------------------------------------------------------
> -
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

sebb-2-2
In reply to this post by Henri Yandell
On 23/11/2009, Henri Yandell <[hidden email]> wrote:
> Given that the Sun JDK's SSL implementation has the same problem as
>  other SSL implementations, and given that Sun don't support the free
>  1.4 and 1.5 versions (so no security fixes coming), I think we do a
>  disservice in even encouraging people to stay on 1.4 and 1.5 by
>  continuing to support them.

I don't see it like that.

The policy has been for Commons components to be upwards compatible,
so IMO the fact that Commons Foo supports Java 1.2 or 1.3 has no
little or no bearing on whether the end user decides to upgrade to
1.6.

Larger organisations tend to be very conservative about upgrading to
the latest version of Java; if we change Commons to require Java 1.6
unneccessarily then IMO the likely effect is to prevent some companies
from using the latest version of Commons, i.e. it makes the situation
slightly worse, not better.

>  1.6 for all Commons components :)

I assume that is not serious, otherwise -1.

I'm all for changing the minimum version of Java where it helps the
end-user, but not just for the sake of it.

>  Hen
>
>
>  On Sun, Nov 22, 2009 at 5:58 PM, Gary Gregory
>  <[hidden email]> wrote:
>  > Our server and tools are all on Java 6, so for us, moving to 6 is quite fine.
>  >
>  > We strongly encourage our users to move away from 1.4.2 ASAP.
>  >
>  > Gary
>  >
>  >> -----Original Message-----
>  >> From: Phil Steitz [mailto:[hidden email]]
>  >> Sent: Sunday, November 22, 2009 14:03
>  >> To: Commons Developers List
>  >> Subject: Re: [dbcp] 1.3 release problems
>  >>
>  >> Gary Gregory wrote:
>  >> > Does dropping support for Java 1.4 simply the issue?
>  >>
>  >> No. Would have to drop 1.5 too.
>  >>
>  >> Phil
>  >> >
>  >> > Gary
>  >> >
>  >> >> -----Original Message-----
>  >> >> From: Phil Steitz [mailto:[hidden email]]
>  >> >> Sent: Sunday, November 22, 2009 08:43
>  >> >> To: Commons Developers List
>  >> >> Subject: [dbcp] 1.3 release problems
>  >> >>
>  >> >> I am running into some problems preparing for dbcp-1.3.  I would
>  >> >> appreciate comments / patches on any of the issues below.
>  >> >>
>  >> >> 1. Findbugs is showing some real (inconsistent synch) and not so
>  >> >> real (e.g. serialization issues on classes that IMO should not be
>  >> >> serializable, but we can't fix until 2.0).  The full report is here:
>  >> >> http://commons.apache.org/dbcp/findbugs.html
>  >> >> I would appreciate suggestions/patches/commits for what to fix and
>  >> how.
>  >> >>
>  >> >> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>  >> >> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>  >> >> as the code stands today).  So we need to create two jar artifacts.
>  >> >>  The question is which one gets the 1.3 name, what is the other
>  >> >> named and how do we package the distros?
>  >> >>
>  >> >> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
>  >> >> and compiler flags for JDBC 2.
>  >> >>
>  >> >> TIA
>  >> >>
>  >> >> Phil
>  >> >>
>  >> >> --------------------------------------------------------------------
>  >> -
>  >> >> To unsubscribe, e-mail: [hidden email]
>  >> >> For additional commands, e-mail: [hidden email]
>  >> >
>  >> >
>  >> > ---------------------------------------------------------------------
>  >> > To unsubscribe, e-mail: [hidden email]
>  >> > For additional commands, e-mail: [hidden email]
>  >> >
>  >>
>  >>
>  >> ---------------------------------------------------------------------
>  >> To unsubscribe, e-mail: [hidden email]
>  >> For additional commands, e-mail: [hidden email]
>  >
>  >
>  > ---------------------------------------------------------------------
>  > To unsubscribe, e-mail: [hidden email]
>  > For additional commands, e-mail: [hidden email]
>  >
>  >
>
>
> ---------------------------------------------------------------------
>
> To unsubscribe, e-mail: [hidden email]
>  For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

sebb-2-2
In reply to this post by Phil Steitz
On 23/11/2009, Phil Steitz <[hidden email]> wrote:

> sebb wrote:
>  > On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>  >> sebb wrote:
>  >>  > On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>  >>  >> I am running into some problems preparing for dbcp-1.3.  I would
>  >>  >>  appreciate comments / patches on any of the issues below.
>  >>  >>
>  >>  >>  1. Findbugs is showing some real (inconsistent synch) and not so
>  >>  >>  real (e.g. serialization issues on classes that IMO should not be
>  >>  >>  serializable, but we can't fix until 2.0).  The full report is here:
>  >>  >>  http://commons.apache.org/dbcp/findbugs.html
>  >>  >>  I would appreciate suggestions/patches/commits for what to fix and how.
>  >>  >
>  >>  > org.apache.commons.dbcp.AbandonedTrace$AbandonedObjectException.format
>  >>  > - not a problem, as the code is synch. on format, just disable the report
>  >>
>  >>
>  >> +1
>  >>
>  >>  > org.apache.commons.dbcp.PoolableConnectionFactory._connFactory,_pool,_validationQuery
>  >>  > => just make these volatile.
>  >>
>  >>
>  >> +1 - all we can do without breaking compat
>  >>
>  >>  > org.apache.commons.dbcp.PoolingConnection.createKey(String, byte)
>  >>  > might ignore java.lang.Exception (lines218, 229, 240 and 251)
>  >>  > No idea
>  >>
>  >>
>  >> This is silly - exceptions potentially thrown by getCatalog are
>  >>  (intentionally) swallowed.
>  >
>  > However the methods should only catch SQLException, not Exception.
>
>
> +1 - and that we can fix.
>
> >
>  > _catalog should probably have been final and private. Or could
>  > probably be dropped altogether, as it does not seem to be necessary.
>
>
> I think the field is necessary - at least there was a BZ ticket that
>  caused it to be added ([Bug 27246] - PreparedStatement cache should
>  be different depending on the Catalog).  Agree it and other key
>  fields should be final.  Unfortunately, they are all protected now,
>  so to fix is to break.

Yes, making protected fields private & final will change part of the API.

However, is that part of the API essential from the user point of view?
I.e. are there use-cases that require it?

> >
>  >>  > PoolingConnection$PStmtKey.PoolingConnection$PStmtKey._resultSetType
>  >>  > could be null and is guaranteed to be dereferenced in
>  >>  > org.apache.commons.dbcp.PoolingConnection.makeObject(Object)
>  >>  > This looks like a bug; just check for null in the second condition?
>  >>
>  >>
>  >> Should never happen, but will refactor to explicitly avoid.
>  >>
>  >>  > Class org.apache.commons.dbcp.cpdsadapter.DriverAdapterCPDS defines
>  >>  > non-transient non-serializable instance field logWriter
>  >>  > Just make the logWriter transient.
>  >>
>  >>
>  >> +1
>  >>
>  >>  > _pool synch: add synch or make volatile.
>  >>
>  >>
>  >> I guess make volatile is safest.
>  >>
>  >>> <aside>
>  >>  > Seems to me a lot of these synch. problems would be avoided if the
>  >>  > variables did not have set() methods - why are there set() methods for
>  >>  > fields that are provided in the constructors? What is the use case for
>  >>  > this?
>  >>  > </aside>
>  >>
>  >>
>  >> Agree strongly with comment.  BasicDataSource is crippled by this.
>  >>  It is effectively immutable once getConnection has been called, but
>  >>  the public setters and protected fields make it impossible to fix
>  >>  without breaking compatibility.  See DBCP-300 for example of how
>  >>  this causes needless performance problems. For Tomcat, I have been
>  >>  thinking about providing an alternative JNDI factory that returns a
>  >>  PoolingDataSource instead.
>  >>
>  >>
>  >>  >
>  >>  > It would be helpful to know which classes are intended to be
>  >>  > thread-safe, as it's not clear whether the potential synch. problems
>  >>  > are likely to occur in normal usage or not.
>  >>  >
>  >>  > For example the class SharedPoolDataSource: the field "pool" is
>  >>  > sometimes synch., and sometimes not, but the fields maxActive,
>  >>  > maxWait, maxIdle are not synch. at all.
>  >>
>  >>
>  >> Here again, all of these should be immutable properties set by the
>  >>  constructor.
>  >>
>  >>  > The use of synchronization seems rather haphazard to me.
>  >>
>  >>
>  >> harsh but true ;)  Comment above really covers it - the needlessly
>  >>  sloppy synch is in most cases due to overly mutable - and sometimes
>  >>  directly exposed - properties and no concern for synch issues that
>  >>  are not likely to occur in normal use.  I am +1 for fixing anything
>  >>  that we can pre-2.0 subject to compat and performance constraints.
>  >>
>  >>  >>  2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>  >>  >>  and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>  >>  >>  as the code stands today).  So we need to create two jar artifacts.
>  >>  >
>  >>  > How difficult would it be to support both in the same jar?
>  >>
>  >>
>  >> I would like to do that if we could do it safely.  I have not been
>  >>  able to get the 1.6-compiled jar to successfully run the tests
>  >>  compiled against 1.5.
>  >>
>  >>  The failure that I get when using Ant to compile and execute the
>  >>  tests (commenting out the 1.6-stuff in the test classes) using a
>  >>  1.6-built jar is strange:
>  >>
>  >>  [junit] Exception in thread "Thread-16"
>  >>  java.lang.NoClassDefFoundError: java/sql/SQLClientInfoException
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:592)
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1537)
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1526)
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1374)
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1038)
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.TestBasicDataSource.getConnection(TestBasicDataSource.java:44)
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84)
>  >>     [junit]     at
>  >>  org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595)
>  >>     [junit]     at java.lang.Thread.run(Thread.java:613)
>  >>
>  >>  Strange as the line number in PCF.makeObject and missing class makes
>  >>  no sense.
>  >>
>  >>
>  >>  Thanks, Sebb!
>  >>
>  >>  >>   The question is which one gets the 1.3 name, what is the other
>  >>  >>  named and how do we package the distros?
>  >>  >>
>  >>  >>  3. I assume it is OK at this point to drop the nojdbc3 Ant target
>  >>  >>  and compiler flags for JDBC 2.
>  >>  >>
>  >>  >>  TIA
>  >>  >>
>  >>  >>  Phil
>  >>  >>
>  >>  >>  ---------------------------------------------------------------------
>  >>  >>  To unsubscribe, e-mail: [hidden email]
>  >>  >>  For additional commands, e-mail: [hidden email]
>  >>  >>
>  >>  >>
>  >>  >
>  >>  > ---------------------------------------------------------------------
>  >>  > To unsubscribe, e-mail: [hidden email]
>  >>  > For additional commands, e-mail: [hidden email]
>  >>  >
>  >>
>  >>
>  >>  ---------------------------------------------------------------------
>  >>  To unsubscribe, e-mail: [hidden email]
>  >>  For additional commands, e-mail: [hidden email]
>  >>
>  >>
>  >
>  > ---------------------------------------------------------------------
>  > To unsubscribe, e-mail: [hidden email]
>  > For additional commands, e-mail: [hidden email]
>  >
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: [hidden email]
>  For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

Phil Steitz
In reply to this post by sebb-2-2
sebb wrote:

> On 23/11/2009, Henri Yandell <[hidden email]> wrote:
>> Given that the Sun JDK's SSL implementation has the same problem as
>>  other SSL implementations, and given that Sun don't support the free
>>  1.4 and 1.5 versions (so no security fixes coming), I think we do a
>>  disservice in even encouraging people to stay on 1.4 and 1.5 by
>>  continuing to support them.
>
> I don't see it like that.
>
> The policy has been for Commons components to be upwards compatible,
> so IMO the fact that Commons Foo supports Java 1.2 or 1.3 has no
> little or no bearing on whether the end user decides to upgrade to
> 1.6.
>
> Larger organisations tend to be very conservative about upgrading to
> the latest version of Java; if we change Commons to require Java 1.6
> unneccessarily then IMO the likely effect is to prevent some companies
> from using the latest version of Commons, i.e. it makes the situation
> slightly worse, not better.
>
>>  1.6 for all Commons components :)
>
> I assume that is not serious, otherwise -1.

Agree with sebb on this.  Dropping support for 1.5 is not a serious
option.  Since 1.4 ~ 1.5 wrt this problem, I want to keep the min
jdk for dbcp 1.3 at jdk 1.4. In dbcp/pool 2.0, we can up the minimum
to 1.5+ and take advantage of the concurrency package and / or
borrow from Oliver's great new concurrency additions to [lang].
There are quite a few bug fixes in dbcp 1.3 that we should make
available ss drop-in replacement for users of earlier versions.

I am being a little inconsistent here by increasing the required
level to 1.4, which normally should only be done in a major version
release, but 1.3 is so old now that I am OK with that change.

Phil

>
> I'm all for changing the minimum version of Java where it helps the
> end-user, but not just for the sake of it.
>
>>  Hen
>>
>>
>>  On Sun, Nov 22, 2009 at 5:58 PM, Gary Gregory
>>  <[hidden email]> wrote:
>>  > Our server and tools are all on Java 6, so for us, moving to 6 is quite fine.
>>  >
>>  > We strongly encourage our users to move away from 1.4.2 ASAP.
>>  >
>>  > Gary
>>  >
>>  >> -----Original Message-----
>>  >> From: Phil Steitz [mailto:[hidden email]]
>>  >> Sent: Sunday, November 22, 2009 14:03
>>  >> To: Commons Developers List
>>  >> Subject: Re: [dbcp] 1.3 release problems
>>  >>
>>  >> Gary Gregory wrote:
>>  >> > Does dropping support for Java 1.4 simply the issue?
>>  >>
>>  >> No. Would have to drop 1.5 too.
>>  >>
>>  >> Phil
>>  >> >
>>  >> > Gary
>>  >> >
>>  >> >> -----Original Message-----
>>  >> >> From: Phil Steitz [mailto:[hidden email]]
>>  >> >> Sent: Sunday, November 22, 2009 08:43
>>  >> >> To: Commons Developers List
>>  >> >> Subject: [dbcp] 1.3 release problems
>>  >> >>
>>  >> >> I am running into some problems preparing for dbcp-1.3.  I would
>>  >> >> appreciate comments / patches on any of the issues below.
>>  >> >>
>>  >> >> 1. Findbugs is showing some real (inconsistent synch) and not so
>>  >> >> real (e.g. serialization issues on classes that IMO should not be
>>  >> >> serializable, but we can't fix until 2.0).  The full report is here:
>>  >> >> http://commons.apache.org/dbcp/findbugs.html
>>  >> >> I would appreciate suggestions/patches/commits for what to fix and
>>  >> how.
>>  >> >>
>>  >> >> 2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>>  >> >> and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>>  >> >> as the code stands today).  So we need to create two jar artifacts.
>>  >> >>  The question is which one gets the 1.3 name, what is the other
>>  >> >> named and how do we package the distros?
>>  >> >>
>>  >> >> 3. I assume it is OK at this point to drop the nojdbc3 Ant target
>>  >> >> and compiler flags for JDBC 2.
>>  >> >>
>>  >> >> TIA
>>  >> >>
>>  >> >> Phil
>>  >> >>
>>  >> >> --------------------------------------------------------------------
>>  >> -
>>  >> >> To unsubscribe, e-mail: [hidden email]
>>  >> >> For additional commands, e-mail: [hidden email]
>>  >> >
>>  >> >
>>  >> > ---------------------------------------------------------------------
>>  >> > To unsubscribe, e-mail: [hidden email]
>>  >> > For additional commands, e-mail: [hidden email]
>>  >> >
>>  >>
>>  >>
>>  >> ---------------------------------------------------------------------
>>  >> To unsubscribe, e-mail: [hidden email]
>>  >> For additional commands, e-mail: [hidden email]
>>  >
>>  >
>>  > ---------------------------------------------------------------------
>>  > To unsubscribe, e-mail: [hidden email]
>>  > For additional commands, e-mail: [hidden email]
>>  >
>>  >
>>
>>
>> ---------------------------------------------------------------------
>>
>> To unsubscribe, e-mail: [hidden email]
>>  For additional commands, e-mail: [hidden email]
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

Phil Steitz
In reply to this post by sebb-2-2
sebb wrote:

> On 23/11/2009, Phil Steitz <[hidden email]> wrote:
>> sebb wrote:
>>  > On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>>  >> sebb wrote:
>>  >>  > On 22/11/2009, Phil Steitz <[hidden email]> wrote:
>>  >>  >> I am running into some problems preparing for dbcp-1.3.  I would
>>  >>  >>  appreciate comments / patches on any of the issues below.
>>  >>  >>
>>  >>  >>  1. Findbugs is showing some real (inconsistent synch) and not so
>>  >>  >>  real (e.g. serialization issues on classes that IMO should not be
>>  >>  >>  serializable, but we can't fix until 2.0).  The full report is here:
>>  >>  >>  http://commons.apache.org/dbcp/findbugs.html
>>  >>  >>  I would appreciate suggestions/patches/commits for what to fix and how.
>>  >>  >
>>  >>  > org.apache.commons.dbcp.AbandonedTrace$AbandonedObjectException.format
>>  >>  > - not a problem, as the code is synch. on format, just disable the report
>>  >>
>>  >>
>>  >> +1
>>  >>
>>  >>  > org.apache.commons.dbcp.PoolableConnectionFactory._connFactory,_pool,_validationQuery
>>  >>  > => just make these volatile.
>>  >>
>>  >>
>>  >> +1 - all we can do without breaking compat
>>  >>
>>  >>  > org.apache.commons.dbcp.PoolingConnection.createKey(String, byte)
>>  >>  > might ignore java.lang.Exception (lines218, 229, 240 and 251)
>>  >>  > No idea
>>  >>
>>  >>
>>  >> This is silly - exceptions potentially thrown by getCatalog are
>>  >>  (intentionally) swallowed.
>>  >
>>  > However the methods should only catch SQLException, not Exception.
>>
>>
>> +1 - and that we can fix.
>>
>>  > _catalog should probably have been final and private. Or could
>>  > probably be dropped altogether, as it does not seem to be necessary.
>>
>>
>> I think the field is necessary - at least there was a BZ ticket that
>>  caused it to be added ([Bug 27246] - PreparedStatement cache should
>>  be different depending on the Catalog).  Agree it and other key
>>  fields should be final.  Unfortunately, they are all protected now,
>>  so to fix is to break.
>
> Yes, making protected fields private & final will change part of the API.
>
> However, is that part of the API essential from the user point of view?
> I.e. are there use-cases that require it?

Not to my knowledge.  Note that the same applies to lots of
protected fields sprinkled throughout dbcp.  This case is probably
more ridiculous than others. From the ciirr report you can see that
we have included a few similar breaks, so I am +0 on fixing this.

Phil

>
>>  >>  > PoolingConnection$PStmtKey.PoolingConnection$PStmtKey._resultSetType
>>  >>  > could be null and is guaranteed to be dereferenced in
>>  >>  > org.apache.commons.dbcp.PoolingConnection.makeObject(Object)
>>  >>  > This looks like a bug; just check for null in the second condition?
>>  >>
>>  >>
>>  >> Should never happen, but will refactor to explicitly avoid.
>>  >>
>>  >>  > Class org.apache.commons.dbcp.cpdsadapter.DriverAdapterCPDS defines
>>  >>  > non-transient non-serializable instance field logWriter
>>  >>  > Just make the logWriter transient.
>>  >>
>>  >>
>>  >> +1
>>  >>
>>  >>  > _pool synch: add synch or make volatile.
>>  >>
>>  >>
>>  >> I guess make volatile is safest.
>>  >>
>>  >>> <aside>
>>  >>  > Seems to me a lot of these synch. problems would be avoided if the
>>  >>  > variables did not have set() methods - why are there set() methods for
>>  >>  > fields that are provided in the constructors? What is the use case for
>>  >>  > this?
>>  >>  > </aside>
>>  >>
>>  >>
>>  >> Agree strongly with comment.  BasicDataSource is crippled by this.
>>  >>  It is effectively immutable once getConnection has been called, but
>>  >>  the public setters and protected fields make it impossible to fix
>>  >>  without breaking compatibility.  See DBCP-300 for example of how
>>  >>  this causes needless performance problems. For Tomcat, I have been
>>  >>  thinking about providing an alternative JNDI factory that returns a
>>  >>  PoolingDataSource instead.
>>  >>
>>  >>
>>  >>  >
>>  >>  > It would be helpful to know which classes are intended to be
>>  >>  > thread-safe, as it's not clear whether the potential synch. problems
>>  >>  > are likely to occur in normal usage or not.
>>  >>  >
>>  >>  > For example the class SharedPoolDataSource: the field "pool" is
>>  >>  > sometimes synch., and sometimes not, but the fields maxActive,
>>  >>  > maxWait, maxIdle are not synch. at all.
>>  >>
>>  >>
>>  >> Here again, all of these should be immutable properties set by the
>>  >>  constructor.
>>  >>
>>  >>  > The use of synchronization seems rather haphazard to me.
>>  >>
>>  >>
>>  >> harsh but true ;)  Comment above really covers it - the needlessly
>>  >>  sloppy synch is in most cases due to overly mutable - and sometimes
>>  >>  directly exposed - properties and no concern for synch issues that
>>  >>  are not likely to occur in normal use.  I am +1 for fixing anything
>>  >>  that we can pre-2.0 subject to compat and performance constraints.
>>  >>
>>  >>  >>  2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>>  >>  >>  and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>>  >>  >>  as the code stands today).  So we need to create two jar artifacts.
>>  >>  >
>>  >>  > How difficult would it be to support both in the same jar?
>>  >>
>>  >>
>>  >> I would like to do that if we could do it safely.  I have not been
>>  >>  able to get the 1.6-compiled jar to successfully run the tests
>>  >>  compiled against 1.5.
>>  >>
>>  >>  The failure that I get when using Ant to compile and execute the
>>  >>  tests (commenting out the 1.6-stuff in the test classes) using a
>>  >>  1.6-built jar is strange:
>>  >>
>>  >>  [junit] Exception in thread "Thread-16"
>>  >>  java.lang.NoClassDefFoundError: java/sql/SQLClientInfoException
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:592)
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1537)
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1526)
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1374)
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1038)
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.TestBasicDataSource.getConnection(TestBasicDataSource.java:44)
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84)
>>  >>     [junit]     at
>>  >>  org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595)
>>  >>     [junit]     at java.lang.Thread.run(Thread.java:613)
>>  >>
>>  >>  Strange as the line number in PCF.makeObject and missing class makes
>>  >>  no sense.
>>  >>
>>  >>
>>  >>  Thanks, Sebb!
>>  >>
>>  >>  >>   The question is which one gets the 1.3 name, what is the other
>>  >>  >>  named and how do we package the distros?
>>  >>  >>
>>  >>  >>  3. I assume it is OK at this point to drop the nojdbc3 Ant target
>>  >>  >>  and compiler flags for JDBC 2.
>>  >>  >>
>>  >>  >>  TIA
>>  >>  >>
>>  >>  >>  Phil
>>  >>  >>
>>  >>  >>  ---------------------------------------------------------------------
>>  >>  >>  To unsubscribe, e-mail: [hidden email]
>>  >>  >>  For additional commands, e-mail: [hidden email]
>>  >>  >>
>>  >>  >>
>>  >>  >
>>  >>  > ---------------------------------------------------------------------
>>  >>  > To unsubscribe, e-mail: [hidden email]
>>  >>  > For additional commands, e-mail: [hidden email]
>>  >>  >
>>  >>
>>  >>
>>  >>  ---------------------------------------------------------------------
>>  >>  To unsubscribe, e-mail: [hidden email]
>>  >>  For additional commands, e-mail: [hidden email]
>>  >>
>>  >>
>>  >
>>  > ---------------------------------------------------------------------
>>  > To unsubscribe, e-mail: [hidden email]
>>  > For additional commands, e-mail: [hidden email]
>>  >
>>
>>
>>  ---------------------------------------------------------------------
>>  To unsubscribe, e-mail: [hidden email]
>>  For additional commands, e-mail: [hidden email]
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [dbcp] 1.3 release problems

Phil Steitz
In reply to this post by Phil Steitz
Phil Steitz wrote:
> sebb wrote:

>> PoolingConnection$PStmtKey.PoolingConnection$PStmtKey._resultSetType
>> could be null and is guaranteed to be dereferenced in
>> org.apache.commons.dbcp.PoolingConnection.makeObject(Object)
>> This looks like a bug; just check for null in the second condition?
>
> Should never happen, but will refactor to explicitly avoid.

The changes below will make Findbugs happy, but will have a slight
performance impact.  Since RTE is thrown in either case, I am
inclined to leave as is (since path should in fact never get executed).

---
src/java/org/apache/commons/dbcp/cpdsadapter/PooledConnectionImpl.java
(revision 883502)
+++
src/java/org/apache/commons/dbcp/cpdsadapter/PooledConnectionImpl.java
(working copy)
@@ -404,7 +404,7 @@
      */
     public Object makeObject(Object obj) throws Exception {
         if (null == obj || !(obj instanceof PStmtKey)) {
-            throw new IllegalArgumentException();
+            throw new IllegalArgumentException("Invalid or null
statement key");
         } else {
             // _openPstmts++;
             PStmtKey key = (PStmtKey)obj;
@@ -421,11 +421,15 @@
                             key, pstmtPool, delegatingConnection);
                 }
             } else {
-                return new PoolablePreparedStatementStub(
-                        connection.prepareStatement(key._sql,
-                        key._resultSetType.intValue(),
-                        key._resultSetConcurrency.intValue()),
-                        key, pstmtPool, delegatingConnection);
+                if (null == key._resultSetType || null ==
key._resultSetConcurrency) {
+                    throw new IllegalArgumentException("Invalid
statement key");
+                } else {
+                    return new PoolablePreparedStatementStub(
+                            connection.prepareStatement(key._sql,
+                                    key._resultSetType.intValue(),
+
key._resultSetConcurrency.intValue()),
+                                    key, pstmtPool,
delegatingConnection);
+                }
             }
         }
     }


>> Class org.apache.commons.dbcp.cpdsadapter.DriverAdapterCPDS defines
>> non-transient non-serializable instance field logWriter
>> Just make the logWriter transient.
>
> +1
>> _pool synch: add synch or make volatile.
>
> I guess make volatile is safest.
>> <aside>
>> Seems to me a lot of these synch. problems would be avoided if the
>> variables did not have set() methods - why are there set() methods for
>> fields that are provided in the constructors? What is the use case for
>> this?
>> </aside>
>
> Agree strongly with comment.  BasicDataSource is crippled by this.
> It is effectively immutable once getConnection has been called, but
> the public setters and protected fields make it impossible to fix
> without breaking compatibility.  See DBCP-300 for example of how
> this causes needless performance problems. For Tomcat, I have been
> thinking about providing an alternative JNDI factory that returns a
> PoolingDataSource instead.
>
>> It would be helpful to know which classes are intended to be
>> thread-safe, as it's not clear whether the potential synch. problems
>> are likely to occur in normal usage or not.
>>
>> For example the class SharedPoolDataSource: the field "pool" is
>> sometimes synch., and sometimes not, but the fields maxActive,
>> maxWait, maxIdle are not synch. at all.
>
> Here again, all of these should be immutable properties set by the
> constructor.
>> The use of synchronization seems rather haphazard to me.
>
> harsh but true ;)  Comment above really covers it - the needlessly
> sloppy synch is in most cases due to overly mutable - and sometimes
> directly exposed - properties and no concern for synch issues that
> are not likely to occur in normal use.  I am +1 for fixing anything
> that we can pre-2.0 subject to compat and performance constraints.
>>>  2. We can't compile commons-pool-1.3.jar against JDK 1.6 (JDBC 4)
>>>  and expect it to work for JDK 1.4/1.5 (JDBC 3) clients (at least not
>>>  as the code stands today).  So we need to create two jar artifacts.
>> How difficult would it be to support both in the same jar?
>
> I would like to do that if we could do it safely.  I have not been
> able to get the 1.6-compiled jar to successfully run the tests
> compiled against 1.5.
>
> The failure that I get when using Ant to compile and execute the
> tests (commenting out the 1.6-stuff in the test classes) using a
> 1.6-built jar is strange:
>
> [junit] Exception in thread "Thread-16"
> java.lang.NoClassDefFoundError: java/sql/SQLClientInfoException
>     [junit] at
> org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:592)
>     [junit] at
> org.apache.commons.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1537)
>     [junit] at
> org.apache.commons.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1526)
>     [junit] at
> org.apache.commons.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1374)
>     [junit] at
> org.apache.commons.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1038)
>     [junit] at
> org.apache.commons.dbcp.TestBasicDataSource.getConnection(TestBasicDataSource.java:44)
>     [junit] at
> org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84)
>     [junit] at
> org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595)
>     [junit] at java.lang.Thread.run(Thread.java:613)
>
> Strange as the line number in PCF.makeObject and missing class makes
> no sense.
>
>
> Thanks, Sebb!
>>>   The question is which one gets the 1.3 name, what is the other
>>>  named and how do we package the distros?
>>>
>>>  3. I assume it is OK at this point to drop the nojdbc3 Ant target
>>>  and compiler flags for JDBC 2.
>>>
>>>  TIA
>>>
>>>  Phil
>>>
>>>  ---------------------------------------------------------------------
>>>  To unsubscribe, e-mail: [hidden email]
>>>  For additional commands, e-mail: [hidden email]
>>>
>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

12