From 3f8bc36788af228fe0943faee75a098dc0c4df8c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Sun, 18 Feb 2024 17:56:22 +0100 Subject: [PATCH] Fix Gradle File leaks (#105597) Fixing a couple of file leaks (and cleaning up one missing try-with-resources). The directory descriptor leaks in particular were leaking massively on every precommit run, to the point where it slows down the whole system and/or we're running into descriptor limits. --- .../internal/conventions/VersionPropertiesLoader.java | 5 +---- .../gradle/internal/test/rest/CopyRestApiTask.java | 9 ++++++--- .../main/java/org/elasticsearch/gradle/LoggedExec.java | 4 +++- .../gradle/testclusters/ElasticsearchNode.java | 4 +++- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/build-conventions/src/main/java/org/elasticsearch/gradle/internal/conventions/VersionPropertiesLoader.java b/build-conventions/src/main/java/org/elasticsearch/gradle/internal/conventions/VersionPropertiesLoader.java index 510a8df41128..17842461636c 100644 --- a/build-conventions/src/main/java/org/elasticsearch/gradle/internal/conventions/VersionPropertiesLoader.java +++ b/build-conventions/src/main/java/org/elasticsearch/gradle/internal/conventions/VersionPropertiesLoader.java @@ -21,11 +21,8 @@ import java.util.Properties; public class VersionPropertiesLoader { static Properties loadBuildSrcVersion(File input, ProviderFactory providerFactory) throws IOException { Properties props = new Properties(); - InputStream is = new FileInputStream(input); - try { + try (InputStream is = new FileInputStream(input)) { props.load(is); - } finally { - is.close(); } loadBuildSrcVersion(props, providerFactory); return props; diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/CopyRestApiTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/CopyRestApiTask.java index 1fbc367804ed..d4b680655a2e 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/CopyRestApiTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/CopyRestApiTask.java @@ -146,13 +146,16 @@ public class CopyRestApiTask extends DefaultTask { try { // check source folder for tests if (sourceResourceDir != null && new File(sourceResourceDir, REST_TEST_PREFIX).exists()) { - return Files.walk(sourceResourceDir.toPath().resolve(REST_TEST_PREFIX)) - .anyMatch(p -> p.getFileName().toString().endsWith("yml")); + try (var files = Files.walk(sourceResourceDir.toPath().resolve(REST_TEST_PREFIX))) { + return files.anyMatch(p -> p.getFileName().toString().endsWith("yml")); + } } // check output for cases where tests are copied programmatically File yamlTestOutputDir = new File(additionalYamlTestsDir.get().getAsFile(), REST_TEST_PREFIX); if (yamlTestOutputDir.exists()) { - return Files.walk(yamlTestOutputDir.toPath()).anyMatch(p -> p.getFileName().toString().endsWith("yml")); + try (var files = Files.walk(yamlTestOutputDir.toPath())) { + return files.anyMatch(p -> p.getFileName().toString().endsWith("yml")); + } } } catch (IOException e) { throw new IllegalStateException(String.format("Error determining if this project [%s] has rest tests.", getProject()), e); diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java b/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java index acb526cf9a3b..4fda91d33211 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java @@ -118,7 +118,9 @@ public abstract class LoggedExec extends DefaultTask implements FileSystemOperat try { // the file may not exist if the command never output anything if (Files.exists(spoolFile.toPath())) { - Files.lines(spoolFile.toPath()).forEach(logger::error); + try (var lines = Files.lines(spoolFile.toPath())) { + lines.forEach(logger::error); + } } } catch (IOException e) { throw new RuntimeException("could not log", e); diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index ce4fd7502f41..31e1cb882305 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -458,7 +458,9 @@ public class ElasticsearchNode implements TestClusterConfiguration { // make sure we always start fresh if (Files.exists(workingDir)) { if (preserveDataDir) { - Files.list(workingDir).filter(path -> path.equals(confPathData) == false).forEach(this::uncheckedDeleteWithRetry); + try (var files = Files.list(workingDir)) { + files.filter(path -> path.equals(confPathData) == false).forEach(this::uncheckedDeleteWithRetry); + } } else { deleteWithRetry(workingDir); }