From a9beeb1d1ea5ffe56c958b6bb02cfa0360766067 Mon Sep 17 00:00:00 2001 From: Tomas Fernandez Lobbe Date: Wed, 8 Jan 2020 11:16:32 -0800 Subject: [PATCH] SOLR-14169: Fix 20 Resource Leak warnings in SolrJ's apache/solr/common --- solr/CHANGES.txt | 2 + .../apache/solr/common/cloud/ZkNodeProps.java | 4 +- .../org/apache/solr/common/util/Utils.java | 8 +-- .../solr/common/util/ContentStreamTest.java | 12 ++-- .../common/util/TestFastJavabinDecoder.java | 60 +++++++++++-------- .../solr/common/util/TestSolrJsonWriter.java | 7 +-- .../common/util/Utf8CharSequenceTest.java | 37 ++++++++---- 7 files changed, 75 insertions(+), 55 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 8c5723b2edd..f9b0b14c71d 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -224,6 +224,8 @@ Other Changes * SOLR-13778: Solrj client will retry requests on SSLException with a suppressed SocketException (very likely a hard-closed socket connection). +* SOLR-14169: Fix 20 Resource Leak warnings in SolrJ's apache/solr/common (Andras Salamon via Tomás Fernández Löbbe) + ================== 8.4.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java index afa7c66a1b0..dfd85ca403b 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java @@ -94,8 +94,8 @@ public class ZkNodeProps implements JSONWriter.Writable { public static ZkNodeProps load(byte[] bytes) { Map props = null; if (bytes[0] == 2) { - try { - props = (Map) new JavaBinCodec().unmarshal(bytes); + try (JavaBinCodec jbc = new JavaBinCodec()) { + props = (Map) jbc.unmarshal(bytes); } catch (IOException e) { throw new RuntimeException("Unable to parse javabin content"); } diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java index fdea6de1f87..fd5eafaadb8 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java @@ -203,10 +203,10 @@ public class Utils { } public static Writer writeJson(Object o, Writer writer, boolean indent) throws IOException { - new SolrJSONWriter(writer) - .setIndent(indent) - .writeObj(o) - .close(); + try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer)) { + jsonWriter.setIndent(indent) + .writeObj(o); + } return writer; } diff --git a/solr/solrj/src/test/org/apache/solr/common/util/ContentStreamTest.java b/solr/solrj/src/test/org/apache/solr/common/util/ContentStreamTest.java index 2fc4f9a7f3f..1a72981d81d 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/ContentStreamTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/ContentStreamTest.java @@ -47,7 +47,7 @@ public class ContentStreamTest extends SolrTestCaseJ4 { public void testFileStream() throws IOException { File file = new File(createTempDir().toFile(), "README"); - try (InputStream is = new SolrResourceLoader().openResource("solrj/README"); + try (SolrResourceLoader srl = new SolrResourceLoader(); InputStream is = srl.openResource("solrj/README"); FileOutputStream os = new FileOutputStream(file)) { assertNotNull(is); IOUtils.copy(is, os); @@ -70,7 +70,7 @@ public class ContentStreamTest extends SolrTestCaseJ4 { public void testFileStreamGZIP() throws IOException { File file = new File(createTempDir().toFile(), "README.gz"); - try (InputStream is = new SolrResourceLoader().openResource("solrj/README"); + try (SolrResourceLoader srl = new SolrResourceLoader(); InputStream is = srl.openResource("solrj/README"); FileOutputStream os = new FileOutputStream(file); GZIPOutputStream zos = new GZIPOutputStream(os)) { IOUtils.copy(is, zos); @@ -95,7 +95,7 @@ public class ContentStreamTest extends SolrTestCaseJ4 { public void testURLStream() throws IOException { File file = new File(createTempDir().toFile(), "README"); - try (InputStream is = new SolrResourceLoader().openResource("solrj/README"); + try (SolrResourceLoader srl = new SolrResourceLoader(); InputStream is = srl.openResource("solrj/README"); FileOutputStream os = new FileOutputStream(file)) { IOUtils.copy(is, os); } @@ -124,7 +124,7 @@ public class ContentStreamTest extends SolrTestCaseJ4 { public void testURLStreamGZIP() throws IOException { File file = new File(createTempDir().toFile(), "README.gz"); - try (InputStream is = new SolrResourceLoader().openResource("solrj/README"); + try (SolrResourceLoader srl = new SolrResourceLoader(); InputStream is = srl.openResource("solrj/README"); FileOutputStream os = new FileOutputStream(file); GZIPOutputStream zos = new GZIPOutputStream(os)) { IOUtils.copy(is, zos); @@ -149,7 +149,7 @@ public class ContentStreamTest extends SolrTestCaseJ4 { public void testURLStreamCSVGZIPExtention() throws IOException { File file = new File(createTempDir().toFile(), "README.CSV.gz"); - try (InputStream is = new SolrResourceLoader().openResource("solrj/README"); + try (SolrResourceLoader srl = new SolrResourceLoader(); InputStream is = srl.openResource("solrj/README"); FileOutputStream os = new FileOutputStream(file); GZIPOutputStream zos = new GZIPOutputStream(os)) { IOUtils.copy(is, zos); @@ -174,7 +174,7 @@ public class ContentStreamTest extends SolrTestCaseJ4 { public void testURLStreamJSONGZIPExtention() throws IOException { File file = new File(createTempDir().toFile(), "README.json.gzip"); - try (InputStream is = new SolrResourceLoader().openResource("solrj/README"); + try (SolrResourceLoader srl = new SolrResourceLoader(); InputStream is = srl.openResource("solrj/README"); FileOutputStream os = new FileOutputStream(file); GZIPOutputStream zos = new GZIPOutputStream(os)) { IOUtils.copy(is, zos); diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestFastJavabinDecoder.java b/solr/solrj/src/test/org/apache/solr/common/util/TestFastJavabinDecoder.java index 79f1b28ecde..ecbdf449856 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/TestFastJavabinDecoder.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/TestFastJavabinDecoder.java @@ -40,32 +40,33 @@ import static org.apache.solr.common.util.Utils.NEW_LINKED_HASHMAP_FUN; public class TestFastJavabinDecoder extends SolrTestCaseJ4 { - public void testTagRead() throws Exception { BinaryRequestWriter.BAOS baos = new BinaryRequestWriter.BAOS(); FastOutputStream faos = FastOutputStream.wrap(baos); - JavaBinCodec codec = new JavaBinCodec(faos, null); - codec.writeVal(10); - codec.writeVal(100); - codec.writeVal("Hello!"); + try (JavaBinCodec codec = new JavaBinCodec(faos, null)) { + codec.writeVal(10); + codec.writeVal(100); + codec.writeVal("Hello!"); + } faos.flushBuffer(); faos.close(); FastInputStream fis = new FastInputStream(null, baos.getbuf(), 0, baos.size()); - FastJavaBinDecoder.StreamCodec scodec = new FastJavaBinDecoder.StreamCodec(fis); - scodec.start(); - Tag tag = scodec.getTag(); - assertEquals(Tag._SINT, tag); - assertEquals(10, scodec.readSmallInt(scodec.dis)); - tag = scodec.getTag(); - assertEquals(Tag._SINT, tag); - assertEquals(100, scodec.readSmallInt(scodec.dis)); - tag = scodec.getTag(); - assertEquals(Tag._STR, tag); - assertEquals("Hello!", scodec.readStr(fis)); + try (FastJavaBinDecoder.StreamCodec scodec = new FastJavaBinDecoder.StreamCodec(fis)) { + scodec.start(); + Tag tag = scodec.getTag(); + assertEquals(Tag._SINT, tag); + assertEquals(10, scodec.readSmallInt(scodec.dis)); + tag = scodec.getTag(); + assertEquals(Tag._SINT, tag); + assertEquals(100, scodec.readSmallInt(scodec.dis)); + tag = scodec.getTag(); + assertEquals(Tag._STR, tag); + assertEquals("Hello!", scodec.readStr(fis)); + } } public void testSimple() throws IOException { @@ -78,10 +79,14 @@ public class TestFastJavabinDecoder extends SolrTestCaseJ4 { Map m = (Map) Utils.fromJSONString(sampleObj); BinaryRequestWriter.BAOS baos = new BinaryRequestWriter.BAOS(); - new JavaBinCodec().marshal(m, baos); - - Map m2 = (Map) new JavaBinCodec().unmarshal(new FastInputStream(null, baos.getbuf(), 0, baos.size())); + try (JavaBinCodec jbc = new JavaBinCodec()) { + jbc.marshal(m, baos); + } + Map m2; + try (JavaBinCodec jbc = new JavaBinCodec()) { + m2 = (Map) jbc.unmarshal(new FastInputStream(null, baos.getbuf(), 0, baos.size())); + } LinkedHashMap fastMap = (LinkedHashMap) new FastJavaBinDecoder() .withInputStream(new FastInputStream(null, baos.getbuf(), 0, baos.size())) .decode(FastJavaBinDecoder.getEntryListener()); @@ -113,7 +118,6 @@ public class TestFastJavabinDecoder extends SolrTestCaseJ4 { ((Map) m2.get("mapk")).remove("k2"); assertEquals(Utils.writeJson(m2, new StringWriter(), true).toString(), Utils.writeJson(newMap, new StringWriter(), true).toString()); - } public void testFastJavabinStreamingDecoder() throws IOException { @@ -121,8 +125,13 @@ public class TestFastJavabinDecoder extends SolrTestCaseJ4 { try (InputStream is = getClass().getResourceAsStream("/solrj/javabin_sample.bin")) { IOUtils.copy(is, baos); } - SimpleOrderedMap o = (SimpleOrderedMap) new JavaBinCodec().unmarshal(baos.toByteArray()); - SolrDocumentList list = (SolrDocumentList) o.get("response"); + + SolrDocumentList list; + try (JavaBinCodec jbc = new JavaBinCodec()) { + SimpleOrderedMap o = (SimpleOrderedMap) jbc.unmarshal(baos.toByteArray()); + list = (SolrDocumentList) o.get("response"); + } + System.out.println(" " + list.getNumFound() + " , " + list.getStart() + " , " + list.getMaxScore()); class Pojo { long _idx; @@ -172,10 +181,7 @@ public class TestFastJavabinDecoder extends SolrTestCaseJ4 { assertEquals((Float) doc.get("price"), pojo.price, 0.001); } }); - parser.processResponse(new FastInputStream(null, baos.getbuf(), 0, baos.size()), null); - - } public void testParsingWithChildDocs() throws IOException { @@ -195,7 +201,9 @@ public class TestFastJavabinDecoder extends SolrTestCaseJ4 { orderedMap.add("response", sdocs); BinaryRequestWriter.BAOS baos = new BinaryRequestWriter.BAOS(); - new JavaBinCodec().marshal(orderedMap, baos); + try (JavaBinCodec jbc = new JavaBinCodec()) { + jbc.marshal(orderedMap, baos); + } boolean[] useListener = new boolean[1]; useListener[0] = true; diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java b/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java index 3dd2cdd15b6..fe2849e8785 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java @@ -42,10 +42,9 @@ public class TestSolrJsonWriter extends SolrTestCaseJ4 { .add("v632")); }); - new SolrJSONWriter(writer) - .setIndent(true) - .writeObj(map) - .close(); + try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer)) { + jsonWriter.setIndent(true).writeObj(map); + } Object o = Utils.fromJSONString(writer.toString()); assertEquals("v1", Utils.getObjectByPath(o, true, "k1")); assertEquals(1l, Utils.getObjectByPath(o, true, "k2")); diff --git a/solr/solrj/src/test/org/apache/solr/common/util/Utf8CharSequenceTest.java b/solr/solrj/src/test/org/apache/solr/common/util/Utf8CharSequenceTest.java index 04832939513..bd45da9af5f 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/Utf8CharSequenceTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/Utf8CharSequenceTest.java @@ -35,9 +35,10 @@ public class Utf8CharSequenceTest extends SolrTestCaseJ4 { ByteArrayUtf8CharSequence utf8 = new ByteArrayUtf8CharSequence(sb.toString()); ByteArrayOutputStream baos = new ByteArrayOutputStream(); byte[] buf = new byte[256]; - FastOutputStream fos = new FastOutputStream(baos, buf, 0); - fos.writeUtf8CharSeq(utf8); - fos.flush(); + try (FastOutputStream fos = new FastOutputStream(baos, buf, 0)) { + fos.writeUtf8CharSeq(utf8); + fos.flush(); + } byte[] result = baos.toByteArray(); ByteArrayUtf8CharSequence utf81 = new ByteArrayUtf8CharSequence(result, 0, result.length); assertTrue(utf81.equals(utf8)); @@ -50,13 +51,17 @@ public class Utf8CharSequenceTest extends SolrTestCaseJ4 { Map m0 = new HashMap(); m0.put("str", utf8); baos.reset(); - new JavaBinCodec().marshal(m0, baos); + try (JavaBinCodec jbc = new JavaBinCodec()) { + jbc.marshal(m0, baos); + } result = baos.toByteArray(); - Map m1 = (Map) new JavaBinCodec() - .setReadStringAsCharSeq(true) - .unmarshal(new ByteArrayInputStream(result)); - utf81 = (ByteArrayUtf8CharSequence) m1.get("str"); - assertTrue(utf81.equals(utf8)); + try (JavaBinCodec jbc = new JavaBinCodec()) { + Map m1 = (Map) jbc + .setReadStringAsCharSeq(true) + .unmarshal(new ByteArrayInputStream(result)); + utf81 = (ByteArrayUtf8CharSequence) m1.get("str"); + assertTrue(utf81.equals(utf8)); + } } public void testUnMarshal() throws IOException { @@ -78,16 +83,22 @@ public class Utf8CharSequenceTest extends SolrTestCaseJ4 { nl.add("key_long", sb.toString()); nl.add("key5", "5" + str); ByteArrayOutputStream baos = new ByteArrayOutputStream(); - new JavaBinCodec().marshal(nl, baos); + try (JavaBinCodec jbc = new JavaBinCodec()) { + jbc.marshal(nl, baos); + } byte[] bytes = baos.toByteArray(); - NamedList nl1 = (NamedList) new JavaBinCodec() - .setReadStringAsCharSeq(true) - .unmarshal(new ByteArrayInputStream( bytes, 0, bytes.length)); + NamedList nl1; + try (JavaBinCodec jbc = new JavaBinCodec()) { + nl1 = (NamedList) jbc + .setReadStringAsCharSeq(true) + .unmarshal(new ByteArrayInputStream(bytes, 0, bytes.length)); + } byte[] buf = ((ByteArrayUtf8CharSequence) nl1.getVal(0)).getBuf(); ByteArrayUtf8CharSequence valLong = (ByteArrayUtf8CharSequence) nl1.get("key_long"); assertFalse(valLong.getBuf() == buf); + for (int i = 1; i < 6; i++) { ByteArrayUtf8CharSequence val = (ByteArrayUtf8CharSequence) nl1.get("key" + i); assertEquals(buf, val.getBuf());