https://issues.apache.org/bugzilla/show_bug.cgi?id=46209


Filip Hanik <[EMAIL PROTECTED]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |WONTFIX




--- Comment #5 from Filip Hanik <[EMAIL PROTECTED]>  2008-11-14 07:12:23 PST ---
>H C IL: There is an apparent infinite recursive loop in 
>org.apache.tomcat.jdbc.pool.DataSourceProxy.setMinIdle(int)
Fixed before the report was filed.

>H V MS: org.apache.tomcat.jdbc.pool.ConnectionPool.log isn't final but should 
>be
Not an issue

>H V MS: org.apache.tomcat.jdbc.pool.DataSourceFactory.log isn't final but 
>should be
Not an issue

>H V MS: 
>org.apache.tomcat.jdbc.pool.DataSourceFactory$DataSourceHandler.methods isn't 
>final but should be
Not an issue

>H V MS: org.apache.tomcat.jdbc.pool.DataSourceProxy.log isn't final but should 
>be
Not an issue

>H V MS: org.apache.tomcat.jdbc.pool.Driver.pooltable isn't final but should be
Not an issue

>H V MS: org.apache.tomcat.jdbc.pool.PooledConnection.log isn't final but 
>should be
Not an issue

>M B ES: Comparison of String objects using == or != in 
>org.apache.tomcat.jdbc.pool.interceptor.ConnectionState.invoke(Object, Method, 
>Object[]) 
Classes, methods, and fields, whether referenced from Java Virtual Machine
instructions or from other constant pool entries, are named using the constant
pool.Relies on the method name being stored in the constant pool, it'd be
interesting to see if that actually wasn't the case.
The spec says "Classes, methods, and fields, whether referenced from Java
Virtual Machine instructions or from other constant pool entries, are named
using the constant pool."


>M B ES: Comparison of String objects using == or != in 
>org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport.process(String[], 
>Method, boolean) 
Classes, methods, and fields, whether referenced from Java Virtual Machine
instructions or from other constant pool entries, are named using the constant
pool.Relies on the method name being stored in the constant pool, it'd be
interesting to see if that actually wasn't the case.
The spec says "Classes, methods, and fields, whether referenced from Java
Virtual Machine instructions or from other constant pool entries, are named
using the constant pool."

>M B It: org.apache.tomcat.jdbc.pool.FairBlockingQueue$FairIterator.next() 
>can't throw NoSuchElementException
Not an issue, although I dont even understand the bug reprot

>M B Nm: The class name org.apache.tomcat.jdbc.pool.DataSource shadows the 
>simple name of implemented interface javax.sql.DataSource
Not an issue

>M B Nm: The class name org.apache.tomcat.jdbc.pool.Driver shadows the simple 
>name of implemented interface java.sql.Driver
Not an issue

>M D DLS: Dead store to exec in 
>org.apache.tomcat.jdbc.pool.PooledConnection.validate(int, String)
Not an issue

>M D Dm: Call to unsupported method 
>org.apache.tomcat.jdbc.pool.FairBlockingQueue.drainTo(Collection, int) in 
>org.apache.tomcat.jdbc.pool.FairBlockingQueue.drainTo(Collection)
Not an issue

>M D NP: Load of known null value in 
>org.apache.tomcat.jdbc.pool.ConnectionPool.borrowConnection()
Not an issue

>M D REC: Exception is caught when Exception is not thrown in 
>org.apache.tomcat.jdbc.pool.ConnectionPool.getConnection()
One would hope so :) not an issue

>M M UL: org.apache.tomcat.jdbc.pool.FairBlockingQueue.poll(long, TimeUnit) 
>does not release lock on all exception paths
>M M UL: org.apache.tomcat.jdbc.pool.FairBlockingQueue.poll(long, TimeUnit) 
>does not release lock on all exception paths
I'd have to challenge that

>M P SIC: Should org.apache.tomcat.jdbc.pool.ConnectionPool$PoolCleaner be a 
>_static_ inner class?
Not an issue

>M V EI2: new 
>org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport$StatementProxy(SlowQueryReport,
> Object, Object[]) may expose internal representation by storing an externally 
>mutable object into SlowQueryReport$StatementProxy.args
not an issue

>M V MS: org.apache.tomcat.jdbc.pool.DataSourceFactory.ALL_PROPERTIES should be 
>package protected
Not an issue

>M X OBL: Method org.apache.tomcat.jdbc.pool.PooledConnection.validate(int, 
>String) may fail to clean up stream or resource of type java.sql.Statement
Not an issue

>There are quite a few cases of class variables that are not final, even though
>they are set in the constructor and not actually changed thereafter.
Not an issue

>Wherever possible, variables should be made final to ensure thread-safety.
If there is a real bug, we would fix it.

>There are some variables that are protected, even though there is already an
>access method for them.
Not an issue

>For example, DataSourceProxy:driver
>In this case, the variable should definitely be private, as access needs to be
>synchronized.
Dont see an issue here either, however, I would switch 

>PooledConnection:counter is volatile, presumably instead of synchronising it.
>However, volatile does not protect against the read-modify-write involved in
>processing the counter++ statement.
Will switch it to an atomic int, although it wont make a difference since the
value is not used anywhere important in the code

>Same applies to PoolProperties:poolCounter
Made to atomic integer

>Also, both of these are protected. I think they should be final.
not an issue

>It would be useful to use Javadoc to state the class thread-safety or
>otherwise, and to add details of how fields are synchronised, e.g. using the
>@GuardedBy annotation.
yes, that would be nice

>It's confusing to have aliases such as:
>final ReentrantLock lock = FairBlockingQueue.this.lock;
>when the code works just the same without.
code practise, to ensure the global lock reference is not set to null. will not
happen here, but but so goes for 90% of this report.

>Some source files still have the following copyright:
>Copyright: Copyright (c) 2008 Filip Hanik
will clean up


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to