You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/08/11 22:38:18 UTC

svn commit: r1617359 - /tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java

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
     @Override
     public void closeInvoked() {
         while (statements.size()>0) {



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


Re: svn commit: r1617359 - /tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java

Posted by Mark Thomas <ma...@apache.org>.
On 11/08/2014 22:14, Konstantin Kolinko wrote:
> 2014-08-12 0:38 GMT+04:00  <ma...@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


Re: svn commit: r1617359 - /tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-08-12 0:38 GMT+04:00  <ma...@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?

What guarantees that WeakReference to that Statement does not return
null? Where is that hard reference that prevents the statement from
being garbage collected?

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

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

(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?
)

>      @Override
>      public void closeInvoked() {
>          while (statements.size()>0) {
>

Best regards,
Konstantin Kolinko

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