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 14:56:43 GMT
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.

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


Mime
View raw message