[jira] [Commented] (SOLR-10778) Ant precommit task WARNINGS about unclosed resources

["Erick Erickson (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-10778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16030619#comment-16030619 ] 

Erick Erickson commented on SOLR-10778:
---------------------------------------

I was looking at this too a little today, and it's tricky. Some things should be changed I think, but it'll be a case-by-case sort of thing. By my count there are 386 warnings in the code base about failing to close something, See the file I'll attach in a minute.

For instance, there's this pattern:

 JavaBinCodec codec = new JavaBinCodec();
    codec.marshal(nl, os);

that generates one of these warnings since JavaBinCodec implements Closeable.

But "codec.marshal()" calls "codec.finish()" which in turn calls "codec.close()" which tests a flag and conditionally calls "codec.finish()" which sets the flag that's checked in "codec.close()" so it doesn't get into an infinite loop. No, I don't want to clarify that....

I think having marshal() have this side-effect is trappy, I'd much rather see a try-with-resources:
try (JavaBinCodec codec = new JavaBinCodec()) {
    codec.marshal(nl, os);
}
and the marshal() code just do it's thing and the code in finish() just be moved to close(). The marshal() code is not robust anyway:
  public void marshal(Object nl, OutputStream os) throws IOException {
    initWrite(os);
    try {
      writeVal(nl);
    } finally {
      finish();
    }
  }

if an error is thrown from initWrite finish (and thus close) won't be called and this _would_ be a resource leak.
****************
I was about to write that one of the classes that has this a lot is IndexWriter and constructs like "RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);" scare me. But looking more closely, almost all of the warnings are in test files and the code that constructs an IndexWriter but doesn't close it so it'd probably be safe to try-with-resources on it (or other). This wouldn't affect the running Solr since it's test code though, so this is largely cosmetic.

*******************

Since there are so many warnings, and since (I'd think) there will be some classes that lend themselves to clean up and some that don't, maybe the best thing to do would be to create sub-jiras for bite-sized chunks and link them here. That way we can have a sanity check for classes that people _know_ are tricky. 

I'll kick one off for JavaBinCodec.

************



> Ant precommit task WARNINGS about unclosed resources
> ----------------------------------------------------
>
>                 Key: SOLR-10778
>                 URL: https://issues.apache.org/jira/browse/SOLR-10778
>             Project: Solr
>          Issue Type: Improvement
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: clients - java
>    Affects Versions: 4.6
>            Reporter: Andrew Musselman
>            Priority: Minor
>
> During precommit we are seeing lots of warnings about resources that aren't being closed, which could pose problems based on chat amongst the team. Log snippet for example:
>     [mkdir] Created dir: /var/folders/5p/6b46rm_94dzc5m8d4v56tds40000gp/T/ecj1165341501
>  [ecj-lint] Compiling 419 source files to /var/folders/5p/6b46rm_94dzc5m8d4v56tds40000gp/T/ecj1165341501
>  [ecj-lint] ----------
>  [ecj-lint] 1. WARNING in /path/to/lucene-solr/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java (at line 920)
>  [ecj-lint]     new LBHttpSolrClient(httpSolrClientBuilder, httpClient, solrServerUrls) :
>  [ecj-lint]     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  [ecj-lint] Resource leak: '<unassigned Closeable value>' is never closed
>  [ecj-lint] ----------
>  [ecj-lint] ----------
>  [ecj-lint] 2. WARNING in /path/to/lucene-solr/solr/solrj/src/java/org/apache/solr/client/solrj/impl/StreamingBinaryResponseParser.java (at line 49)
>  [ecj-lint]     JavaBinCodec codec = new JavaBinCodec() {
>  [ecj-lint]                  ^^^^^
>  [ecj-lint] Resource leak: 'codec' is never closed
>  [ecj-lint] ----------
>  [ecj-lint] ----------
>  [ecj-lint] 3. WARNING in /path/to/lucene-solr/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java (at line 90)
>  [ecj-lint]     JavaBinCodec codec = new JavaBinCodec();
>  [ecj-lint]                  ^^^^^
>  [ecj-lint] Resource leak: 'codec' is never closed
>  [ecj-lint] ----------
>  [ecj-lint] 4. WARNING in /path/to/lucene-solr/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java (at line 113)
>  [ecj-lint]     JavaBinCodec codec = new JavaBinCodec() {
>  [ecj-lint]                  ^^^^^
>  [ecj-lint] Resource leak: 'codec' is never closed



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

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