Skip to content

Commit bfe436e

Browse files
authored
Fix file handle leak from ProcessBuilder.redirectError() on Windows (#7346)
ProcessBuilder.redirectError(Redirect.appendTo(file)) opens a parent-side file handle that is not released after CreateProcess() on Windows. The leaked handle prevents the log file from being deleted even after the child process terminates. Replace the OS-level stderr redirect with a daemon thread that reads from the child's stderr pipe and writes to the log file. This keeps the file handle lifecycle under JVM control.
1 parent 24938cf commit bfe436e

1 file changed

Lines changed: 32 additions & 9 deletions

File tree

rewrite-core/src/main/java/org/openrewrite/rpc/RewriteRpcProcess.java

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,14 @@
3131
import lombok.Setter;
3232
import org.jspecify.annotations.Nullable;
3333

34-
import java.io.File;
3534
import java.io.IOException;
3635
import java.io.InputStream;
36+
import java.io.OutputStream;
3737
import java.io.UncheckedIOException;
38+
import java.nio.file.Files;
3839
import java.nio.file.Path;
3940
import java.nio.file.Paths;
41+
import java.nio.file.StandardOpenOption;
4042
import java.util.LinkedHashMap;
4143
import java.util.Map;
4244
import java.util.concurrent.TimeUnit;
@@ -47,9 +49,6 @@
4749
* A client for spawning and communicating with a subprocess that implements Rewrite RPC.
4850
*/
4951
public class RewriteRpcProcess extends Thread {
50-
private static final File DEV_NULL = new File(
51-
System.getProperty("os.name").startsWith("Windows") ? "NUL" : "/dev/null");
52-
5352
private final String[] command;
5453

5554
@Setter
@@ -93,17 +92,41 @@ public void run() {
9392
if (workingDirectory != null) {
9493
pb.directory(workingDirectory.toFile());
9594
}
96-
if (stderrRedirect != null) {
97-
pb.redirectError(ProcessBuilder.Redirect.appendTo(stderrRedirect.toFile()));
98-
} else {
99-
pb.redirectError(ProcessBuilder.Redirect.to(DEV_NULL));
100-
}
95+
// Don't use ProcessBuilder.redirectError() — on Windows it leaks the
96+
// parent-side file handle after process termination, preventing deletion
97+
// of the log file. Instead we drain stderr in a daemon thread.
10198
process = pb.start();
99+
drainStderr(process, stderrRedirect);
102100
} catch (IOException e) {
103101
throw new UncheckedIOException(e);
104102
}
105103
}
106104

105+
private static void drainStderr(Process process, @Nullable Path stderrRedirect) {
106+
Thread thread = new Thread(() -> {
107+
byte[] buf = new byte[8192];
108+
try (InputStream stderr = process.getErrorStream()) {
109+
if (stderrRedirect != null) {
110+
try (OutputStream out = Files.newOutputStream(stderrRedirect,
111+
StandardOpenOption.CREATE, StandardOpenOption.APPEND)) {
112+
int n;
113+
while ((n = stderr.read(buf)) != -1) {
114+
out.write(buf, 0, n);
115+
}
116+
}
117+
} else {
118+
//noinspection StatementWithEmptyBody
119+
while (stderr.read(buf) != -1) {
120+
// discard
121+
}
122+
}
123+
} catch (IOException ignored) {
124+
}
125+
}, "rpc-stderr-drain");
126+
thread.setDaemon(true);
127+
thread.start();
128+
}
129+
107130
public @Nullable RuntimeException getLivenessCheck() {
108131
if (process == null) {
109132
return null;

0 commit comments

Comments
 (0)