Replace several try-finally statements (#30880)

This change replaces some existing try-finally statements that close resources
in their finally block with the slightly shorter and safer try-with-resources
pattern.

server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java

@@ -18,12 +18,9 @@
  */
 package org.elasticsearch.common.geo.parsers;
 
-import org.locationtech.jts.geom.Coordinate;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoShapeType;
-
-import java.io.StringReader;
 import org.elasticsearch.common.geo.builders.CoordinatesBuilder;
 import org.elasticsearch.common.geo.builders.EnvelopeBuilder;
 import org.elasticsearch.common.geo.builders.GeometryCollectionBuilder;
@@ -37,9 +34,11 @@ import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.mapper.GeoShapeFieldMapper;
+import org.locationtech.jts.geom.Coordinate;
 
 import java.io.IOException;
 import java.io.StreamTokenizer;
+import java.io.StringReader;
 import java.util.List;
 
 /**
@@ -77,8 +76,7 @@ public class GeoWKTParser {
     public static ShapeBuilder parseExpectedType(XContentParser parser, final GeoShapeType shapeType,
                                                  final GeoShapeFieldMapper shapeMapper)
             throws IOException, ElasticsearchParseException {
-        StringReader reader = new StringReader(parser.text());
-        try {
+        try (StringReader reader = new StringReader(parser.text())) {
             boolean ignoreZValue = (shapeMapper != null && shapeMapper.ignoreZValue().value() == true);
             // setup the tokenizer; configured to read words w/o numbers
             StreamTokenizer tokenizer = new StreamTokenizer(reader);
@@ -95,8 +93,6 @@ public class GeoWKTParser {
             ShapeBuilder builder = parseGeometry(tokenizer, shapeType, ignoreZValue);
             checkEOF(tokenizer);
             return builder;
-        } finally {
-            reader.close();
         }
     }
 

server/src/main/java/org/elasticsearch/index/analysis/Analysis.java

@@ -234,8 +234,8 @@ public class Analysis {
 
         final Path path = env.configFile().resolve(wordListPath);
 
-        try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
-            return loadWordList(reader, "#");
+        try {
+            return loadWordList(path, "#");
         } catch (CharacterCodingException ex) {
             String message = String.format(Locale.ROOT,
                 "Unsupported character encoding detected while reading %s_path: %s - files must be UTF-8 encoded",
@@ -247,15 +247,9 @@ public class Analysis {
         }
     }
 
-    public static List<String> loadWordList(Reader reader, String comment) throws IOException {
+    private static List<String> loadWordList(Path path, String comment) throws IOException {
         final List<String> result = new ArrayList<>();
-        BufferedReader br = null;
-        try {
-            if (reader instanceof BufferedReader) {
-                br = (BufferedReader) reader;
-            } else {
-                br = new BufferedReader(reader);
-            }
+        try (BufferedReader br = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
             String word;
             while ((word = br.readLine()) != null) {
                 if (!Strings.hasText(word)) {
@@ -265,9 +259,6 @@ public class Analysis {
                     result.add(word.trim());
                 }
             }
-        } finally {
-            if (br != null)
-                br.close();
         }
         return result;
     }

server/src/main/java/org/elasticsearch/index/engine/Engine.java

@@ -1424,10 +1424,6 @@ public abstract class Engine implements Closeable {
 
         @Override
         public void close() {
-            release();
-        }
-
-        public void release() {
             Releasables.close(searcher);
         }
     }

server/src/main/java/org/elasticsearch/index/get/ShardGetService.java

@@ -159,7 +159,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
                 get = indexShard.get(new Engine.Get(realtime, readFromTranslog, type, id, uidTerm)
                         .version(version).versionType(versionType));
                 if (get.exists() == false) {
-                    get.release();
+                    get.close();
                 }
             }
         }
@@ -172,7 +172,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
             // break between having loaded it from translog (so we only have _source), and having a document to load
             return innerGetLoadFromStoredFields(type, id, gFields, fetchSourceContext, get, mapperService);
         } finally {
-            get.release();
+            get.close();
         }
     }
 

server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java

@@ -85,8 +85,6 @@ public class TermVectorsService  {
             termVectorsResponse.setExists(false);
             return termVectorsResponse;
         }
-        Engine.GetResult get = indexShard.get(new Engine.Get(request.realtime(), false, request.type(), request.id(), uidTerm)
-                .version(request.version()).versionType(request.versionType()));
 
         Fields termVectorsByField = null;
         AggregatedDfs dfs = null;
@@ -97,8 +95,9 @@ public class TermVectorsService  {
             handleFieldWildcards(indexShard, request);
         }
 
-        final Engine.Searcher searcher = indexShard.acquireSearcher("term_vector");
-        try {
+        try (Engine.GetResult get = indexShard.get(new Engine.Get(request.realtime(), false, request.type(), request.id(), uidTerm)
+                .version(request.version()).versionType(request.versionType()));
+                Engine.Searcher searcher = indexShard.acquireSearcher("term_vector")) {
             Fields topLevelFields = MultiFields.getFields(get.searcher() != null ? get.searcher().reader() : searcher.reader());
             DocIdAndVersion docIdAndVersion = get.docIdAndVersion();
             /* from an artificial document */
@@ -143,14 +142,12 @@ public class TermVectorsService  {
                     }
                 }
                 // write term vectors
-                termVectorsResponse.setFields(termVectorsByField, request.selectedFields(), request.getFlags(), topLevelFields, dfs, termVectorsFilter);
+                termVectorsResponse.setFields(termVectorsByField, request.selectedFields(), request.getFlags(), topLevelFields, dfs,
+                        termVectorsFilter);
             }
             termVectorsResponse.setTookInMillis(TimeUnit.NANOSECONDS.toMillis(nanoTimeSupplier.getAsLong() - startTime));
         } catch (Exception ex) {
             throw new ElasticsearchException("failed to execute term vector request", ex);
-        } finally {
-            searcher.close();
-            get.release();
         }
         return termVectorsResponse;
     }

server/src/main/java/org/elasticsearch/search/SearchService.java

@@ -21,7 +21,6 @@ package org.elasticsearch.search;
 
 import org.apache.lucene.search.FieldDoc;
 import org.apache.lucene.search.TopDocs;
-import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.action.ActionListener;
@@ -39,6 +38,7 @@ import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
 import org.elasticsearch.common.util.concurrent.ConcurrentMapLong;
+import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.IndexSettings;
@@ -92,8 +92,8 @@ import org.elasticsearch.search.sort.SortAndFormats;
 import org.elasticsearch.search.sort.SortBuilder;
 import org.elasticsearch.search.suggest.Suggest;
 import org.elasticsearch.search.suggest.completion.CompletionSuggestion;
-import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.threadpool.Scheduler.Cancellable;
+import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.threadpool.ThreadPool.Names;
 import org.elasticsearch.transport.TransportRequest;
 
@@ -646,20 +646,17 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
 
 
     public boolean freeContext(long id) {
-        final SearchContext context = removeContext(id);
-        if (context != null) {
-            assert context.refCount() > 0 : " refCount must be > 0: " + context.refCount();
-            try {
+        try (SearchContext context = removeContext(id)) {
+            if (context != null) {
+                assert context.refCount() > 0 : " refCount must be > 0: " + context.refCount();
                 context.indexShard().getSearchOperationListener().onFreeContext(context);
                 if (context.scrollContext() != null) {
                     context.indexShard().getSearchOperationListener().onFreeScrollContext(context);
                 }
-            } finally {
-                context.close();
+                return true;
             }
-            return true;
+            return false;
         }
-        return false;
     }
 
     public void freeAllScrollContexts() {

server/src/test/java/org/elasticsearch/action/termvectors/MultiTermVectorsIT.java

@@ -23,6 +23,7 @@ import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.index.Fields;
 import org.apache.lucene.index.Terms;
 import org.apache.lucene.index.TermsEnum;
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.admin.indices.alias.Alias;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.common.settings.Settings;
@@ -111,7 +112,8 @@ public class MultiTermVectorsIT extends AbstractTermVectorsTestCase {
         checkTermTexts(response.getResponses()[1].getResponse().getFields().terms("field"), new String[]{"value1"});
         assertThat(response.getResponses()[2].getFailure(), notNullValue());
         assertThat(response.getResponses()[2].getFailure().getId(), equalTo("1"));
-        assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(VersionConflictEngineException.class));
+        assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(ElasticsearchException.class));
+        assertThat(response.getResponses()[2].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class));
 
         //Version from Lucene index
         refresh();
@@ -132,7 +134,8 @@ public class MultiTermVectorsIT extends AbstractTermVectorsTestCase {
         checkTermTexts(response.getResponses()[1].getResponse().getFields().terms("field"), new String[]{"value1"});
         assertThat(response.getResponses()[2].getFailure(), notNullValue());
         assertThat(response.getResponses()[2].getFailure().getId(), equalTo("1"));
-        assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(VersionConflictEngineException.class));
+        assertThat(response.getResponses()[2].getFailure().getCause(), instanceOf(ElasticsearchException.class));
+        assertThat(response.getResponses()[2].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class));
 
 
         for (int i = 0; i < 3; i++) {
@@ -155,7 +158,8 @@ public class MultiTermVectorsIT extends AbstractTermVectorsTestCase {
         assertThat(response.getResponses()[1].getFailure(), notNullValue());
         assertThat(response.getResponses()[1].getFailure().getId(), equalTo("2"));
         assertThat(response.getResponses()[1].getIndex(), equalTo("test"));
-        assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(VersionConflictEngineException.class));
+        assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(ElasticsearchException.class));
+        assertThat(response.getResponses()[1].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class));
         assertThat(response.getResponses()[2].getId(), equalTo("2"));
         assertThat(response.getResponses()[2].getIndex(), equalTo("test"));
         assertThat(response.getResponses()[2].getFailure(), nullValue());
@@ -180,7 +184,8 @@ public class MultiTermVectorsIT extends AbstractTermVectorsTestCase {
         assertThat(response.getResponses()[1].getFailure(), notNullValue());
         assertThat(response.getResponses()[1].getFailure().getId(), equalTo("2"));
         assertThat(response.getResponses()[1].getIndex(), equalTo("test"));
-        assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(VersionConflictEngineException.class));
+        assertThat(response.getResponses()[1].getFailure().getCause(), instanceOf(ElasticsearchException.class));
+        assertThat(response.getResponses()[1].getFailure().getCause().getCause(), instanceOf(VersionConflictEngineException.class));
         assertThat(response.getResponses()[2].getId(), equalTo("2"));
         assertThat(response.getResponses()[2].getIndex(), equalTo("test"));
         assertThat(response.getResponses()[2].getFailure(), nullValue());

server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java

@@ -21,6 +21,7 @@ package org.elasticsearch.index.engine;
 
 import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
 import com.carrotsearch.randomizedtesting.generators.RandomNumbers;
+
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -793,7 +794,7 @@ public class InternalEngineTests extends EngineTestCase {
             while (flushFinished.get() == false) {
                 Engine.GetResult previousGetResult = latestGetResult.get();
                 if (previousGetResult != null) {
-                    previousGetResult.release();
+                    previousGetResult.close();
                 }
                 latestGetResult.set(engine.get(newGet(true, doc), searcherFactory));
                 if (latestGetResult.get().exists() == false) {
@@ -807,7 +808,7 @@ public class InternalEngineTests extends EngineTestCase {
         flushFinished.set(true);
         getThread.join();
         assertTrue(latestGetResult.get().exists());
-        latestGetResult.get().release();
+        latestGetResult.get().close();
     }
 
     public void testSimpleOperations() throws Exception {
@@ -830,21 +831,20 @@ public class InternalEngineTests extends EngineTestCase {
         searchResult.close();
 
         // but, not there non realtime
-        Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory);
-        assertThat(getResult.exists(), equalTo(false));
-        getResult.release();
+        try (Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory)) {
+            assertThat(getResult.exists(), equalTo(false));
+        }
 
         // but, we can still get it (in realtime)
-        getResult = engine.get(newGet(true, doc), searcherFactory);
-        assertThat(getResult.exists(), equalTo(true));
-        assertThat(getResult.docIdAndVersion(), notNullValue());
-        getResult.release();
+        try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) {
+            assertThat(getResult.exists(), equalTo(true));
+            assertThat(getResult.docIdAndVersion(), notNullValue());
+        }
 
         // but not real time is not yet visible
-        getResult = engine.get(newGet(false, doc), searcherFactory);
-        assertThat(getResult.exists(), equalTo(false));
-        getResult.release();
-
+        try (Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory)) {
+            assertThat(getResult.exists(), equalTo(false));
+        }
 
         // refresh and it should be there
         engine.refresh("test");
@@ -856,10 +856,10 @@ public class InternalEngineTests extends EngineTestCase {
         searchResult.close();
 
         // also in non realtime
-        getResult = engine.get(newGet(false, doc), searcherFactory);
-        assertThat(getResult.exists(), equalTo(true));
-        assertThat(getResult.docIdAndVersion(), notNullValue());
-        getResult.release();
+        try (Engine.GetResult getResult = engine.get(newGet(false, doc), searcherFactory)) {
+            assertThat(getResult.exists(), equalTo(true));
+            assertThat(getResult.docIdAndVersion(), notNullValue());
+        }
 
         // now do an update
         document = testDocument();
@@ -876,10 +876,10 @@ public class InternalEngineTests extends EngineTestCase {
         searchResult.close();
 
         // but, we can still get it (in realtime)
-        getResult = engine.get(newGet(true, doc), searcherFactory);
-        assertThat(getResult.exists(), equalTo(true));
-        assertThat(getResult.docIdAndVersion(), notNullValue());
-        getResult.release();
+        try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) {
+            assertThat(getResult.exists(), equalTo(true));
+            assertThat(getResult.docIdAndVersion(), notNullValue());
+        }
 
         // refresh and it should be updated
         engine.refresh("test");
@@ -901,9 +901,9 @@ public class InternalEngineTests extends EngineTestCase {
         searchResult.close();
 
         // but, get should not see it (in realtime)
-        getResult = engine.get(newGet(true, doc), searcherFactory);
-        assertThat(getResult.exists(), equalTo(false));
-        getResult.release();
+        try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) {
+            assertThat(getResult.exists(), equalTo(false));
+        }
 
         // refresh and it should be deleted
         engine.refresh("test");
@@ -941,10 +941,10 @@ public class InternalEngineTests extends EngineTestCase {
         engine.flush();
 
         // and, verify get (in real time)
-        getResult = engine.get(newGet(true, doc), searcherFactory);
-        assertThat(getResult.exists(), equalTo(true));
-        assertThat(getResult.docIdAndVersion(), notNullValue());
-        getResult.release();
+        try (Engine.GetResult getResult = engine.get(newGet(true, doc), searcherFactory)) {
+            assertThat(getResult.exists(), equalTo(true));
+            assertThat(getResult.docIdAndVersion(), notNullValue());
+        }
 
         // make sure we can still work with the engine
         // now do an update
@@ -4156,7 +4156,7 @@ public class InternalEngineTests extends EngineTestCase {
                     new Term("_id", parsedDocument.id()),
                     parsedDocument,
                     SequenceNumbers.UNASSIGNED_SEQ_NO,
-                    (long) randomIntBetween(1, 8),
+                    randomIntBetween(1, 8),
                     Versions.MATCH_ANY,
                     VersionType.INTERNAL,
                     Engine.Operation.Origin.PRIMARY,
@@ -4172,7 +4172,7 @@ public class InternalEngineTests extends EngineTestCase {
                     id,
                     new Term("_id", parsedDocument.id()),
                     SequenceNumbers.UNASSIGNED_SEQ_NO,
-                    (long) randomIntBetween(1, 8),
+                    randomIntBetween(1, 8),
                     Versions.MATCH_ANY,
                     VersionType.INTERNAL,
                     Engine.Operation.Origin.PRIMARY,

server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java

@@ -1861,10 +1861,11 @@ public class IndexShardTests extends IndexShardTestCase {
         indexDoc(shard, "_doc", "1", "{\"foobar\" : \"bar\"}");
         shard.refresh("test");
 
-        Engine.GetResult getResult = shard.get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1"))));
-        assertTrue(getResult.exists());
-        assertNotNull(getResult.searcher());
-        getResult.release();
+        try (Engine.GetResult getResult = shard
+                .get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1"))))) {
+            assertTrue(getResult.exists());
+            assertNotNull(getResult.searcher());
+        }
         try (Engine.Searcher searcher = shard.acquireSearcher("test")) {
             TopDocs search = searcher.searcher().search(new TermQuery(new Term("foo", "bar")), 10);
             assertEquals(search.totalHits, 1);
@@ -1895,11 +1896,12 @@ public class IndexShardTests extends IndexShardTestCase {
             search = searcher.searcher().search(new TermQuery(new Term("foobar", "bar")), 10);
             assertEquals(search.totalHits, 1);
         }
-        getResult = newShard.get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1"))));
-        assertTrue(getResult.exists());
-        assertNotNull(getResult.searcher()); // make sure get uses the wrapped reader
-        assertTrue(getResult.searcher().reader() instanceof FieldMaskingReader);
-        getResult.release();
+        try (Engine.GetResult getResult = newShard
+                .get(new Engine.Get(false, false, "test", "1", new Term(IdFieldMapper.NAME, Uid.encodeId("1"))))) {
+            assertTrue(getResult.exists());
+            assertNotNull(getResult.searcher()); // make sure get uses the wrapped reader
+            assertTrue(getResult.searcher().reader() instanceof FieldMaskingReader);
+        }
 
         closeShards(newShard);
     }