From 43c3824a4fee08a9acf42898db025647bfac9220 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Fri, 21 Jan 2022 14:47:37 +0100 Subject: [PATCH] Guard against IO resource leak with streams --- .../trino/server/ServerPluginsProvider.java | 11 ++++++---- .../BenchmarkGeometryAggregations.java | 12 +++++++---- .../geospatial/BenchmarkSpatialJoin.java | 12 +++++++---- .../geospatial/TestBingTileFunctions.java | 6 +++++- .../geospatial/TestSphericalGeoFunctions.java | 21 ++++++++++++------- pom.xml | 1 + .../launcher/cli/EnvironmentDescribe.java | 11 ++++++---- 7 files changed, 50 insertions(+), 24 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/server/ServerPluginsProvider.java b/core/trino-main/src/main/java/io/trino/server/ServerPluginsProvider.java index 45c1f5e43cac..733bb13f15c2 100644 --- a/core/trino-main/src/main/java/io/trino/server/ServerPluginsProvider.java +++ b/core/trino-main/src/main/java/io/trino/server/ServerPluginsProvider.java @@ -22,6 +22,7 @@ import java.io.UncheckedIOException; import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.DirectoryStream; import java.nio.file.Path; import java.util.List; import java.util.concurrent.Callable; @@ -70,10 +71,12 @@ private static List buildClassPath(File path) private static List listFiles(File path) { try { - return stream(newDirectoryStream(path.toPath())) - .map(Path::toFile) - .sorted() - .collect(toImmutableList()); + try (DirectoryStream directoryStream = newDirectoryStream(path.toPath())) { + return stream(directoryStream) + .map(Path::toFile) + .sorted() + .collect(toImmutableList()); + } } catch (IOException e) { throw new UncheckedIOException(e); diff --git a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkGeometryAggregations.java b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkGeometryAggregations.java index c520c9706919..48ca6fcbc55c 100644 --- a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkGeometryAggregations.java +++ b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkGeometryAggregations.java @@ -32,6 +32,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.google.common.io.Resources.getResource; import static io.trino.jmh.Benchmarks.benchmark; @@ -70,10 +71,13 @@ public void setUp() queryRunner.createCatalog("memory", new MemoryConnectorFactory(), ImmutableMap.of()); Path path = new File(getResource("us-states.tsv").toURI()).toPath(); - String polygonValues = Files.lines(path) - .map(line -> line.split("\t")) - .map(parts -> format("('%s', '%s')", parts[0], parts[1])) - .collect(Collectors.joining(",")); + String polygonValues; + try (Stream lines = Files.lines(path)) { + polygonValues = lines + .map(line -> line.split("\t")) + .map(parts -> format("('%s', '%s')", parts[0], parts[1])) + .collect(Collectors.joining(",")); + } queryRunner.execute( format("CREATE TABLE memory.default.us_states AS SELECT ST_GeometryFromText(t.wkt) AS geom FROM (VALUES %s) as t (name, wkt)", diff --git a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java index f3af448c158b..cc047b7c5014 100644 --- a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java +++ b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/BenchmarkSpatialJoin.java @@ -38,6 +38,7 @@ import java.nio.file.Path; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.google.common.io.Resources.getResource; import static io.trino.jmh.Benchmarks.benchmark; @@ -82,10 +83,13 @@ public void setUp() queryRunner.createCatalog("memory", new MemoryConnectorFactory(), ImmutableMap.of()); Path path = new File(getResource("us-states.tsv").toURI()).toPath(); - String polygonValues = Files.lines(path) - .map(line -> line.split("\t")) - .map(parts -> format("('%s', '%s')", parts[0], parts[1])) - .collect(Collectors.joining(",")); + String polygonValues; + try (Stream lines = Files.lines(path)) { + polygonValues = lines + .map(line -> line.split("\t")) + .map(parts -> format("('%s', '%s')", parts[0], parts[1])) + .collect(Collectors.joining(",")); + } queryRunner.execute(format("CREATE TABLE memory.default.polygons AS SELECT * FROM (VALUES %s) as t (name, wkt)", polygonValues)); } diff --git a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestBingTileFunctions.java b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestBingTileFunctions.java index 350b26401e6d..c53de92d23cb 100644 --- a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestBingTileFunctions.java +++ b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestBingTileFunctions.java @@ -25,6 +25,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; +import java.util.stream.Stream; import static com.google.common.io.Resources.getResource; import static io.trino.operator.scalar.ApplyFunction.APPLY_FUNCTION; @@ -455,7 +456,10 @@ public void testGeometryToBingTiles() // Input polygon too complex String filePath = new File(getResource("too_large_polygon.txt").toURI()).getPath(); - String largeWkt = Files.lines(Paths.get(filePath)).findFirst().get(); + String largeWkt; + try (Stream lines = Files.lines(Paths.get(filePath))) { + largeWkt = lines.findFirst().get(); + } assertInvalidFunction("geometry_to_bing_tiles(ST_GeometryFromText('" + largeWkt + "'), 16)", "The zoom level is too high or the geometry is too complex to compute a set of covering Bing tiles. Please use a lower zoom level or convert the geometry to its bounding box using the ST_Envelope function."); assertFunction("cardinality(geometry_to_bing_tiles(ST_Envelope(ST_GeometryFromText('" + largeWkt + "')), 16))", BIGINT, 19939L); diff --git a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestSphericalGeoFunctions.java b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestSphericalGeoFunctions.java index 0d90af1f9971..aba391391777 100644 --- a/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestSphericalGeoFunctions.java +++ b/plugin/trino-geospatial/src/test/java/io/trino/plugin/geospatial/TestSphericalGeoFunctions.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.google.common.io.Resources.getResource; import static io.airlift.slice.Slices.utf8Slice; @@ -178,15 +179,21 @@ public void testArea() assertArea("POLYGON((90 0, 0 0, 0 90), (89 1, 1 1, 1 89))", 348.04E10); Path geometryPath = new File(getResource("us-states.tsv").toURI()).toPath(); - Map stateGeometries = Files.lines(geometryPath) - .map(line -> line.split("\t")) - .collect(Collectors.toMap(parts -> parts[0], parts -> parts[1])); + Map stateGeometries; + try (Stream lines = Files.lines(geometryPath)) { + stateGeometries = lines + .map(line -> line.split("\t")) + .collect(Collectors.toMap(parts -> parts[0], parts -> parts[1])); + } Path areaPath = new File(getResource("us-state-areas.tsv").toURI()).toPath(); - Map stateAreas = Files.lines(areaPath) - .map(line -> line.split("\t")) - .filter(parts -> parts.length >= 2) - .collect(Collectors.toMap(parts -> parts[0], parts -> Double.valueOf(parts[1]))); + Map stateAreas; + try (Stream lines = Files.lines(areaPath)) { + stateAreas = lines + .map(line -> line.split("\t")) + .filter(parts -> parts.length >= 2) + .collect(Collectors.toMap(parts -> parts[0], parts -> Double.valueOf(parts[1]))); + } for (String state : stateGeometries.keySet()) { assertArea(stateGeometries.get(state), stateAreas.get(state)); diff --git a/pom.xml b/pom.xml index 2ad442573bc3..b6eec55a3014 100644 --- a/pom.xml +++ b/pom.xml @@ -1848,6 +1848,7 @@ -Xep:MutablePublicArray:ERROR \ -Xep:NullOptional:ERROR \ -Xep:ObjectToString:ERROR \ + -Xep:StreamResourceLeak:ERROR \ -Xep:UnnecessaryMethodReference:ERROR \ -Xep:UnnecessaryOptionalGet:ERROR \ -Xep:UnusedVariable:ERROR \ diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java index 2c4474d9d941..268cd33a1de9 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java @@ -45,6 +45,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.Callable; +import java.util.stream.Stream; import static io.trino.tests.product.launcher.cli.Commands.runCommand; import static io.trino.tests.product.launcher.docker.DockerFiles.ROOT_PATH; @@ -201,10 +202,12 @@ private String simplifyPath(String path) private static long directorySize(Path directory) { try { - return Files.walk(directory) - .filter(path -> path.toFile().isFile()) - .mapToLong(path -> path.toFile().length()) - .sum(); + try (Stream stream = Files.walk(directory)) { + return stream + .filter(path -> path.toFile().isFile()) + .mapToLong(path -> path.toFile().length()) + .sum(); + } } catch (IOException e) { log.warn(e, "Could not calculate directory size: %s", directory);