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] 1.3 release problems
Date Tue, 24 Nov 2009 23:37:17 GMT
sebb wrote:
> On 24/11/2009, sebb <sebbaz@gmail.com> wrote:
>> On 24/11/2009, Phil Steitz <phil.steitz@gmail.com> wrote:
>>  > Phil Steitz wrote:
>>  >  > sebb wrote:
>>  >  >> On 22/11/2009, Phil Steitz <phil.steitz@gmail.com> 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.
>>  >  >
>>  >
>>  > This ^^^ is bugging me as I don't see why it shouldn't work.  I just
>>  >  committed an Ant build file, "test-jar.xml" that compiles and runs
>>  >  the tests against a compiled jar.  Could be something wrong with my
>>  >  local setup, so I would appreciate it if others could test using JDK
>>  >  1.5, 1.4.
>>
>>
>> Created dist/commons-dbpc.jar using "ant dist" under Java 1.6.0_17 (WinXP)
>>
>>  ant -f test-jar.xml  clean test -Dcp=dist/commons-dbcp.jar
>>
>>  generates a lot of stack trace output but test succeeds.
>>
>>  I then switched to Java 1.5.0_22
>>
>>  ant -f test-jar.xml  clean test -Dcp=dist/commons-dbcp.jar
>>
>>  fails with:
>>
>>  compile-test:
>>     [mkdir] Created dir:
>>  D:\eclipseworkspaces\main\commons-dbcp-rw\build\test-classes
>>     [javac] Compiling 39 source files to
>>  D:\eclipseworkspaces\main\commons-dbcp-rw\build\test-classes
>>     [javac] D:\eclipseworkspaces\main\commons-dbcp-rw\build\src\test\org\apache\commons\dbcp\TestBasicDataSource.java:47:
>>  cannot access org.apache.com
>>  mons.dbcp.BasicDataSource
>>     [javac] bad class file:
>>  D:\eclipseworkspaces\main\commons-dbcp-rw\dist\commons-dbcp.jar(org/apache/commons/dbcp/BasicDataSource.class)
>>     [javac] class file has wrong version 50.0, should be 49.0
>>     [javac] Please remove or make sure it appears in the correct
>>  subdirectory of the classpath.
>>     [javac]     protected BasicDataSource ds = null;
>>     [javac]               ^
>>     [javac] 1 error
>>
>>  This is presumably because the main build.xml file does not define the
>>  target Java version, so it defaults to 1.6.
>>
>>  Or am I going about the test in the wrong way?
>>
>>  I'll try changing the main build java target and see what happens.
>>
> 
> Using Java 1.6 to compile with target=1.5 works fine.
> 
> However, the test fails for me with the same error when testing the
> jar using Java 1.5.
> 
> I think the error at the following line:
> 
> 592: return new PoolableConnection(conn,_pool,_config);
> 
> occurs because PoolableConnection extends DelegatingConnection which
> imports java.sql.SQLClientInfoException (this is one of the imports
> that are for JDBC4 only)

Thanks.  That's the piece that I was missing. I conclude from this
that we really are not going to be able to produce one binary jar
that works for all supported JDKs.  I will post a proposal shortly
for packaging strategy.

Thanks for getting the fixes in so the sources now compile clean for
 1.6-1.6.

Phil
> 
> It's not possible to test on Java 1.4 at present because there is some
> test code that requires Java 1.5 to build.
> 
> Note that DBCP builds and tests OK for me using Java 1.5 throughout.
> 
> 
>>  >
>>  >  Phil
>>  >
>>  > >
>>  >  > 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: 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
>>  >  >>
>>  >  >
>>  >
>>  >
>>  >  ---------------------------------------------------------------------
>>  >  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
> 


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


Mime
View raw message