tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1617359 - /tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
Date Mon, 11 Aug 2014 21:27:17 GMT
On 11/08/2014 22:14, Konstantin Kolinko wrote:
> 2014-08-12 0:38 GMT+04:00  <markt@apache.org>:
>> Author: markt
>> Date: Mon Aug 11 20:38:18 2014
>> New Revision: 1617359
>>
>> URL: http://svn.apache.org/r1617359
>> Log:
>> Silence some unnecessary nags
>>
>> Modified:
>>     tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
>>
>> Modified: tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java?rev=1617359&r1=1617358&r2=1617359&view=diff
>> ==============================================================================
>> --- tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
(original)
>> +++ tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java
Mon Aug 11 20:38:18 2014
>> @@ -52,6 +52,7 @@ public class StatementFinalizer extends
>>          return statement;
>>      }
>>
>> +    @SuppressWarnings("null") // st is not null when used
> 
> Where it is used?

boolean shallClose = false;
try {
    shallClose = st!=null && (!st.isClosed());
    if (shallClose) {
        st.close();   <=== Possible NPE warning

The IDE doesn't tale account of the lines above that mean st.close()
only ever gets called when st is non-null.

> What guarantees that WeakReference to that Statement does not return
> null?

Not relevant.

> Where is that hard reference that prevents the statement from
> being garbage collected?

Also not relevant.

> With the current code I think ws.getStatement() can return null.

No one is disagreeing with you.

> I am starting to think that we do not need a WeakReference for a
> Statement, but a usual hard reference would work better.

I haven't looked at the code other than the three lines above which were
enough to convince me that the IDE warning was a false positive.

> (Just saying, without digging into the code. The relevant questions are
> - Can it be that the only hard reference to Statement object are in user's code?
> - If yes, then what happens if user clears those references and allows
> the Statement to be GC'ed? Are java.sql.Statement implementations
> expected to implement finalize() method that calls close() on them?
> )

None of those points seem relevant to my commit. I haven't dug deeper to
look at the wider code. I was just looking at the IDE warning.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message