Skip to content

Commit

Permalink
fix(fd): close leaked file descriptors (#1445)
Browse files Browse the repository at this point in the history 
* fix(fd): close leaked file descriptors

* bump -core version
  • Loading branch information
@andrewazores
andrewazores committed Apr 12, 2023
1 parent 741dbbe commit ef36883
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 84 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
<com.google.dagger.version>2.45</com.google.dagger.version>
<com.google.dagger.compiler.version>2.45</com.google.dagger.compiler.version>

<io.cryostat.core.version>2.19.1</io.cryostat.core.version>
<io.cryostat.core.version>2.19.2</io.cryostat.core.version>

<org.openjdk.nashorn.core.version>15.4</org.openjdk.nashorn.core.version>
<org.apache.commons.lang3.version>3.12.0</org.apache.commons.lang3.version>
Expand Down
18 changes: 6 additions & 12 deletions src/main/java/io/cryostat/net/openshift/OpenShiftNetworkModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
*/
package io.cryostat.net.openshift;

import java.io.BufferedReader;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -80,12 +81,8 @@ public abstract class OpenShiftNetworkModule {
value = "DMI_HARDCODED_ABSOLUTE_FILENAME",
justification = "Kubernetes namespace file path is well-known and absolute")
static String provideNamespace(FileSystem fs) {
try {
return fs.readFile(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))
.lines()
.filter(StringUtils::isNotBlank)
.findFirst()
.get();
try (BufferedReader br = fs.readFile(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))) {
return br.lines().filter(StringUtils::isNotBlank).findFirst().get();
} catch (IOException e) {
throw new RuntimeException(e);
}
Expand All @@ -98,12 +95,9 @@ static String provideNamespace(FileSystem fs) {
value = "DMI_HARDCODED_ABSOLUTE_FILENAME",
justification = "Kubernetes serviceaccount file path is well-known and absolute")
static String provideServiceAccountToken(FileSystem fs) {
try {
return fs.readFile(Paths.get(Config.KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH))
.lines()
.filter(StringUtils::isNotBlank)
.findFirst()
.get();
try (BufferedReader br =
fs.readFile(Paths.get(Config.KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH))) {
return br.lines().filter(StringUtils::isNotBlank).findFirst().get();
} catch (IOException e) {
throw new RuntimeException(e);
}
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/io/cryostat/net/reports/SubprocessReportGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
*/
package io.cryostat.net.reports;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.InvocationTargetException;
Expand Down Expand Up @@ -156,11 +157,12 @@ public synchronized CompletableFuture<Path> exec(
: ExitStatus.byExitCode(proc.exitValue());

if (fs.exists(reportStatsPath)) {
ReportStats stats =
gson.fromJson(fs.readFile(reportStatsPath), ReportStats.class);
evt.setRecordingSizeBytes(stats.getRecordingSizeBytes());
evt.setRulesEvaluated(stats.getRulesEvaluated());
evt.setRulesApplicable(stats.getRulesApplicable());
try (BufferedReader br = fs.readFile(reportStatsPath)) {
ReportStats stats = gson.fromJson(br, ReportStats.class);
evt.setRecordingSizeBytes(stats.getRecordingSizeBytes());
evt.setRulesEvaluated(stats.getRulesEvaluated());
evt.setRulesApplicable(stats.getRulesApplicable());
}
}

switch (status) {
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
package io.cryostat.recordings;

import java.io.BufferedInputStream;
import java.io.BufferedReader;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
Expand Down Expand Up @@ -354,8 +355,9 @@ protected Future<String> getConnectUrlFromPath(Path subdirectory) {
for (String file : fs.listDirectoryChildren(subdirectory)) {
// use metadata file to determine connectUrl to probe for jvmId
if (file.equals(CONNECT_URL)) {
connectUrl =
Optional.of(fs.readFile(subdirectory.resolve(file)).readLine());
try (BufferedReader r = fs.readFile(subdirectory.resolve(file))) {
connectUrl = Optional.of(r.readLine());
}
}
}
future.complete(connectUrl.orElseThrow(IOException::new));
Expand Down
136 changes: 73 additions & 63 deletions src/main/java/io/cryostat/recordings/RecordingMetadataManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

package io.cryostat.recordings;

import java.io.BufferedReader;
import java.io.IOException;
import java.lang.reflect.Type;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -213,58 +214,65 @@ public void start(Promise<Void> future) {
.filter(Objects::nonNull)
.forEach(
pair -> {
StoredRecordingMetadata srm =
gson.fromJson(
pair.getLeft(),
StoredRecordingMetadata
.class);
Path file = pair.getRight();
String targetId = srm.getTargetId();
String recordingName =
srm.getRecordingName();
// jvmId should always exist
// since we are using directory
// structure
if (srm.getJvmId() != null) {
try {
if (!isArchivedRecording(
recordingName)) {
logger.info(
"Potentially"
+ " stale"
+ " metadata"
+ " file:"
+ " {}, for"
+ " target:"
try (BufferedReader br =
pair.getLeft()) {
StoredRecordingMetadata srm =
gson.fromJson(
br,
StoredRecordingMetadata
.class);
Path file = pair.getRight();
String targetId =
srm.getTargetId();
String recordingName =
srm.getRecordingName();
// jvmId should always exist
// since we are using directory
// structure
if (srm.getJvmId() != null) {
try {
if (!isArchivedRecording(
recordingName)) {
logger.info(
"Potentially"
+ " stale"
+ " metadata"
+ " file:"
+ " {}, for"
+ " target:"
+ " {}",
recordingName,
targetId);
staleMetadata.put(
srm, file);
return;
}
} catch (IOException e) {
logger.error(
"Could not"
+ " check"
+ " if recording"
+ " {} exists"
+ " on target"
+ " {}, msg:"
+ " {}",
recordingName,
targetId);
staleMetadata.put(
srm, file);
return;
targetId,
e.getMessage());
}
} catch (IOException e) {
logger.error(
"Could not check if"
+ " recording"
+ " {} exists"
+ " on target"
+ " {}, msg:"
+ " {}",
recordingName,
targetId,
e.getMessage());
}
} else {
logger.warn(
"Invalid metadata with"
+ " no jvmId"
} else {
logger.warn(
"Invalid metadata"
+ " with no"
+ " jvmId"
+ " originating"
+ " from"
+ " {}",
targetId);
deleteMetadataPathIfExists(
file);
+ " from {}",
targetId);
deleteMetadataPathIfExists(
file);
}
} catch (IOException ioe) {
logger.error(ioe);
}
});
}
Expand All @@ -280,11 +288,8 @@ public void start(Promise<Void> future) {
(remove after 2.2.0 release and replace with subdirectory::fs.isDirectory (ignore files))? */
else if (fs.isRegularFile(subdirectory)) {
StoredRecordingMetadata srm;
try {
srm =
gson.fromJson(
fs.readFile(subdirectory),
StoredRecordingMetadata.class);
try (BufferedReader br = fs.readFile(subdirectory)) {
srm = gson.fromJson(br, StoredRecordingMetadata.class);
} catch (Exception e) {
logger.error(
"Could not read file {} in recordingMetadata"
Expand Down Expand Up @@ -626,7 +631,9 @@ public Metadata getMetadata(ConnectionDescriptor connectionDescriptor, String re
metadata = new Metadata();
fs.writeString(metadataPath, gson.toJson(metadata));
} else {
metadata = gson.fromJson(fs.readFile(metadataPath), Metadata.class);
try (BufferedReader br = fs.readFile(metadataPath)) {
metadata = gson.fromJson(br, Metadata.class);
}
}
return metadata;
}
Expand All @@ -642,7 +649,9 @@ public Metadata getMetadataFromPathIfExists(String jvmId, String recordingName)
fs.writeString(metadataPath, gson.toJson(metadata));
return metadata;
}
return gson.fromJson(fs.readFile(metadataPath), Metadata.class);
try (BufferedReader br = fs.readFile(metadataPath)) {
return gson.fromJson(br, Metadata.class);
}
}

public Metadata deleteRecordingMetadataIfExists(
Expand All @@ -661,11 +670,13 @@ public Metadata deleteRecordingMetadataIfExists(String jvmId, String recordingNa

Path metadataPath = this.getMetadataPath(jvmId, recordingName);
if (fs.isRegularFile(metadataPath)) {
Metadata metadata = gson.fromJson(fs.readFile(metadataPath), Metadata.class);
if (fs.deleteIfExists(metadataPath)) {
deleteSubdirectoryIfEmpty(metadataPath.getParent());
try (BufferedReader br = fs.readFile(metadataPath)) {
Metadata metadata = gson.fromJson(br, Metadata.class);
if (fs.deleteIfExists(metadataPath)) {
deleteSubdirectoryIfEmpty(metadataPath.getParent());
}
return metadata;
}
return metadata;
}
return null;
}
Expand Down Expand Up @@ -740,11 +751,10 @@ private void transferMetadataIfRestarted(ConnectionDescriptor cd) {
logger.info("[{}] Metadata transfer: {} -> {}", targetId, oldJvmId, newJvmId);
Path oldParent = getMetadataPath(oldJvmId);
for (String encodedFilename : fs.listDirectoryChildren(oldParent)) {
try {
Path oldMetadataPath = oldParent.resolve(encodedFilename);
Path oldMetadataPath = oldParent.resolve(encodedFilename);
try (BufferedReader storedMetadata = fs.readFile(oldMetadataPath)) {
StoredRecordingMetadata srm =
gson.fromJson(
fs.readFile(oldMetadataPath), StoredRecordingMetadata.class);
gson.fromJson(storedMetadata, StoredRecordingMetadata.class);
String recordingName = srm.recordingName;
StoredRecordingMetadata updatedSrm =
StoredRecordingMetadata.of(targetId, newJvmId, recordingName, srm);
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/io/cryostat/rules/RuleRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,16 @@ public void loadRules() throws IOException {
}
})
.filter(Objects::nonNull)
.map(reader -> gson.fromJson(reader, Rule.class))
.map(
reader -> {
try (reader) {
return gson.fromJson(reader, Rule.class);
} catch (IOException ioe) {
logger.error(ioe);
return null;
}
})
.filter(Objects::nonNull)
.forEach(rules::add);
}

Expand Down

0 comments on commit ef36883

Please sign in to comment.