Skip to content

Commit ac62ae7

Browse files
committed
Fix file descriptor leaks in ServerStatusManager and ZooKeeperCommandExecutor
Motivation: `Files.newInputStream()` and `Files.newOutputStream()` allocate real OS file descriptors. `Properties.load()` and `Properties.store()` do not close the stream passed to them, so the caller is responsible for closing it. The descriptors are eventually reclaimed by GC, but GC is triggered by memory pressure rather than FD exhaustion. Modifications: - `ServerStatusManager.serverStatus()`: wrap `Files.newInputStream()` in try-with-resources so the stream is closed after `Properties.load()`. - `ServerStatusManager.updateStatus()`: same fix for the load path; also wrap `Files.newOutputStream()` in try-with-resources so the stream is closed after `Properties.store()`. Result: - File descriptors opened for property file I/O are now released deterministically instead of relying on GC finalisation.
1 parent 99db9ec commit ac62ae7

2 files changed

Lines changed: 11 additions & 15 deletions

File tree

server/src/main/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutor.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -772,19 +772,14 @@ private static boolean shutdown(@Nullable ExecutorService executor) {
772772
}
773773

774774
private long getLastReplayedRevision() throws Exception {
775-
final FileInputStream fis;
776-
try {
777-
fis = new FileInputStream(revisionFile);
778-
} catch (FileNotFoundException ignored) {
779-
return -1;
780-
}
781-
782-
try (BufferedReader br = new BufferedReader(new InputStreamReader(fis))) {
775+
try (BufferedReader br = new BufferedReader(new InputStreamReader(new FileInputStream(revisionFile)))) {
783776
final String l = br.readLine();
784777
if (l == null) {
785778
return -1;
786779
}
787780
return Long.parseLong(l.trim());
781+
} catch (FileNotFoundException ignored) {
782+
return -1;
788783
}
789784
}
790785

server/src/main/java/com/linecorp/centraldogma/server/management/ServerStatusManager.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
import java.io.File;
2222
import java.io.IOException;
23+
import java.io.InputStream;
24+
import java.io.OutputStream;
2325
import java.nio.file.Files;
2426
import java.nio.file.Path;
2527
import java.util.Properties;
@@ -65,8 +67,8 @@ public ServerStatusManager(File dataDir) {
6567
public ServerStatus serverStatus() {
6668
final Properties properties = new Properties();
6769
synchronized (serverStatusFile) {
68-
try {
69-
properties.load(Files.newInputStream(serverStatusFile));
70+
try (InputStream in = Files.newInputStream(serverStatusFile)) {
71+
properties.load(in);
7072
} catch (IOException e) {
7173
throw new IllegalStateException("Failed to load server status file: " + serverStatusFile, e);
7274
}
@@ -85,16 +87,15 @@ public ServerStatus serverStatus() {
8587
public void updateStatus(ServerStatus newServerStatus) {
8688
synchronized (serverStatusFile) {
8789
final Properties properties = new Properties();
88-
try {
89-
properties.load(Files.newInputStream(serverStatusFile));
90+
try (InputStream in = Files.newInputStream(serverStatusFile)) {
91+
properties.load(in);
9092
} catch (IOException e) {
9193
throw new IllegalStateException("Failed to load server status file: " + serverStatusFile,
9294
e);
9395
}
9496
properties.setProperty(STATUS, newServerStatus.name());
95-
try {
96-
properties.store(Files.newOutputStream(serverStatusFile),
97-
"Do not edit this file manually. Use the AdministrativeService API.");
97+
try (OutputStream out = Files.newOutputStream(serverStatusFile)) {
98+
properties.store(out, "Do not edit this file manually. Use the AdministrativeService API.");
9899
} catch (IOException e) {
99100
throw new IllegalStateException("Failed to store server status file: " + serverStatusFile,
100101
e);

0 commit comments

Comments
 (0)