[
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: [email protected]
For additional commands, e-mail: [email protected]
