mirror of
https://github.com/DerDavidBohl/dirigent-spring.git
synced 2026-01-06 08:49:41 -06:00
Refactor ProcessRunner to remove process tracking and cleanup logic, simplifying command execution
This commit is contained in:
@@ -8,82 +8,16 @@ import java.nio.charset.StandardCharsets;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.ScheduledExecutorService;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
import jakarta.annotation.PreDestroy;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
@Component
|
||||
@Slf4j
|
||||
public class ProcessRunner {
|
||||
|
||||
// Track active processes to ensure cleanup
|
||||
private final Map<Process, Long> activeProcesses = new ConcurrentHashMap<>();
|
||||
|
||||
// Background thread to clean up any stale processes
|
||||
private final ScheduledExecutorService cleanupExecutor = Executors.newSingleThreadScheduledExecutor(r -> {
|
||||
Thread t = new Thread(r, "process-cleanup-thread");
|
||||
t.setDaemon(true);
|
||||
return t;
|
||||
});
|
||||
|
||||
public ProcessRunner() {
|
||||
// Start periodic cleanup task to catch any missed processes
|
||||
cleanupExecutor.scheduleAtFixedRate(this::cleanupStaleProcesses, 10, 10, TimeUnit.SECONDS);
|
||||
log.info("ProcessRunner initialized with background cleanup thread");
|
||||
}
|
||||
|
||||
@PreDestroy
|
||||
public void shutdown() {
|
||||
log.info("Shutting down ProcessRunner and cleaning up {} remaining processes", activeProcesses.size());
|
||||
cleanupStaleProcesses();
|
||||
cleanupExecutor.shutdown();
|
||||
try {
|
||||
if (!cleanupExecutor.awaitTermination(5, TimeUnit.SECONDS)) {
|
||||
cleanupExecutor.shutdownNow();
|
||||
}
|
||||
} catch (InterruptedException e) {
|
||||
cleanupExecutor.shutdownNow();
|
||||
Thread.currentThread().interrupt();
|
||||
}
|
||||
}
|
||||
|
||||
private void cleanupStaleProcesses() {
|
||||
long now = System.currentTimeMillis();
|
||||
activeProcesses.entrySet().removeIf(entry -> {
|
||||
Process process = entry.getKey();
|
||||
long startTime = entry.getValue();
|
||||
|
||||
// Check if process is still alive
|
||||
if (!process.isAlive()) {
|
||||
log.debug("Removing dead process from tracking (age: {}s)", (now - startTime) / 1000);
|
||||
return true;
|
||||
}
|
||||
|
||||
// If process has been running for more than 2 minutes, kill it
|
||||
if ((now - startTime) > 120000) {
|
||||
log.warn("Killing stale process (age: {}s)", (now - startTime) / 1000);
|
||||
process.destroyForcibly();
|
||||
try {
|
||||
process.waitFor(5, TimeUnit.SECONDS);
|
||||
} catch (InterruptedException e) {
|
||||
Thread.currentThread().interrupt();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
});
|
||||
|
||||
if (!activeProcesses.isEmpty()) {
|
||||
log.debug("Active processes being tracked: {}", activeProcesses.size());
|
||||
}
|
||||
}
|
||||
|
||||
public ProcessResult executeCommand(List<String> commandParts, long timeoutMs, Map<String, String> env)
|
||||
throws IOException {
|
||||
return executeInternal(commandParts, new File(System.getProperty("user.dir")), timeoutMs, env);
|
||||
@@ -114,14 +48,13 @@ public class ProcessRunner {
|
||||
|
||||
Map<String, String> finalEnv = new HashMap<>();
|
||||
finalEnv.putAll(System.getenv());
|
||||
|
||||
if(env != null && !env.isEmpty())
|
||||
if(env != null && !env.isEmpty()) {
|
||||
finalEnv.putAll(env);
|
||||
}
|
||||
|
||||
ProcessBuilder processBuilder = new ProcessBuilder(commandParts);
|
||||
processBuilder.directory(workingDirectory);
|
||||
processBuilder.environment().putAll(finalEnv);
|
||||
processBuilder.redirectErrorStream(false);
|
||||
|
||||
log.debug("Running command <{}> in directory {}", String.join(" ", commandParts), workingDirectory);
|
||||
|
||||
@@ -133,56 +66,28 @@ public class ProcessRunner {
|
||||
try {
|
||||
process = processBuilder.start();
|
||||
|
||||
// Track this process immediately
|
||||
activeProcesses.put(process, System.currentTimeMillis());
|
||||
|
||||
// Read stdout in separate thread
|
||||
Thread stdoutReader = new Thread(() -> {
|
||||
try (BufferedReader reader = new BufferedReader(
|
||||
new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) {
|
||||
String line;
|
||||
while ((line = reader.readLine()) != null) {
|
||||
stdout.append(line).append("\n");
|
||||
}
|
||||
} catch (IOException e) {
|
||||
log.debug("Error reading stdout", e);
|
||||
}
|
||||
}, "stdout-reader");
|
||||
stdoutReader.setDaemon(true);
|
||||
stdoutReader.start();
|
||||
// Read stdout
|
||||
Thread stdoutReader = readStream(process.getInputStream(), stdout);
|
||||
// Read stderr
|
||||
Thread stderrReader = readStream(process.getErrorStream(), stderr);
|
||||
|
||||
// Read stderr in separate thread
|
||||
Thread stderrReader = new Thread(() -> {
|
||||
try (BufferedReader reader = new BufferedReader(
|
||||
new InputStreamReader(process.getErrorStream(), StandardCharsets.UTF_8))) {
|
||||
String line;
|
||||
while ((line = reader.readLine()) != null) {
|
||||
stderr.append(line).append("\n");
|
||||
}
|
||||
} catch (IOException e) {
|
||||
log.debug("Error reading stderr", e);
|
||||
}
|
||||
}, "stderr-reader");
|
||||
stderrReader.setDaemon(true);
|
||||
stderrReader.start();
|
||||
|
||||
// Wait for process with timeout
|
||||
// Wait for process to complete (1 minute timeout)
|
||||
boolean finished = process.waitFor(1, TimeUnit.MINUTES);
|
||||
|
||||
if (!finished) {
|
||||
log.warn("Process timed out, killing it: {}", String.join(" ", commandParts));
|
||||
log.warn("Process timed out: {}", String.join(" ", commandParts));
|
||||
process.destroyForcibly();
|
||||
process.waitFor(5, TimeUnit.SECONDS);
|
||||
}
|
||||
|
||||
// Wait for reader threads to finish
|
||||
// Wait for output streams to finish reading
|
||||
stdoutReader.join(2000);
|
||||
stderrReader.join(2000);
|
||||
|
||||
exitCode = process.exitValue();
|
||||
|
||||
} catch (InterruptedException e) {
|
||||
log.warn("Process got interrupted: {}", String.join(" ", commandParts), e);
|
||||
log.warn("Process interrupted: {}", String.join(" ", commandParts), e);
|
||||
if (process != null && process.isAlive()) {
|
||||
process.destroyForcibly();
|
||||
try {
|
||||
@@ -193,30 +98,37 @@ public class ProcessRunner {
|
||||
}
|
||||
Thread.currentThread().interrupt();
|
||||
} finally {
|
||||
// CRITICAL: Always remove from tracking and ensure process is dead
|
||||
if (process != null) {
|
||||
activeProcesses.remove(process);
|
||||
|
||||
// Double-check the process is actually dead
|
||||
if (process.isAlive()) {
|
||||
log.warn("Process still alive in finally block, force killing");
|
||||
process.destroyForcibly();
|
||||
try {
|
||||
process.waitFor(5, TimeUnit.SECONDS);
|
||||
} catch (InterruptedException e) {
|
||||
Thread.currentThread().interrupt();
|
||||
}
|
||||
// Ensure process is terminated
|
||||
if (process != null && process.isAlive()) {
|
||||
log.warn("Force killing remaining process");
|
||||
process.destroyForcibly();
|
||||
try {
|
||||
process.waitFor(5, TimeUnit.SECONDS);
|
||||
} catch (InterruptedException e) {
|
||||
Thread.currentThread().interrupt();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
String stdoutString = stdout.toString().trim();
|
||||
String stderrString = stderr.toString().trim();
|
||||
log.debug("Finished command <{}> with exit code {}", String.join(" ", commandParts), exitCode);
|
||||
|
||||
log.debug("Finished command <{}>\nExit code: <{}>\nstdout: {}\nstderr: {}",
|
||||
String.join(" ", commandParts), exitCode, stdoutString, stderrString);
|
||||
return new ProcessResult(exitCode, stdout.toString().trim(), stderr.toString().trim());
|
||||
}
|
||||
|
||||
return new ProcessResult(exitCode, stdoutString, stderrString);
|
||||
private Thread readStream(java.io.InputStream inputStream, StringBuilder output) {
|
||||
Thread reader = new Thread(() -> {
|
||||
try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
|
||||
String line;
|
||||
while ((line = br.readLine()) != null) {
|
||||
output.append(line).append("\n");
|
||||
}
|
||||
} catch (IOException e) {
|
||||
log.debug("Error reading stream", e);
|
||||
}
|
||||
});
|
||||
reader.setDaemon(true);
|
||||
reader.start();
|
||||
return reader;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user