On 24/11/2009, sebb <seb...@gmail.com> wrote: > On 24/11/2009, sebb <seb...@gmail.com> wrote: > > On 24/11/2009, Phil Steitz <phil.ste...@gmail.com> wrote: > > > Phil Steitz wrote: > > > > sebb wrote: > > > >> On 22/11/2009, Phil Steitz <phil.ste...@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. >
I've now fixed the code so that it clean builds and tests on 1.4, 1.5 and 1.6. [there were some Java 1.5 methods used in the JDBC3 code sections] It also appears that the Java 1.4 code is upwards compatible: I built and tested the code on Java 1.4, and then tested on 1.5 and 1.6 - all tests succeeded, but of course none of the JDBC4 tests were enabled. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
