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.
>
> 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-unsubscr...@commons.apache.org
> >>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>
> >>>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >> For additional commands, e-mail: dev-h...@commons.apache.org
> >>
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
