commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [dbcp] 1.3 release problems
Date Tue, 24 Nov 2009 18:13:57 GMT
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)

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


Mime
View raw message