Skip to content

Commit b9090f9

Browse files
author
Mark Pollack
committed
Avoid ForkJoinPool.commonPool in closeGracefully() per best practices
1 parent ba3709d commit b9090f9

File tree

1 file changed

+28
-20
lines changed

1 file changed

+28
-20
lines changed

acp-core/src/main/java/com/agentclientprotocol/sdk/client/transport/StdioAcpClientTransport.java

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -371,39 +371,47 @@ public Mono<Void> closeGracefully() {
371371
inboundSink.tryEmitComplete();
372372
outboundSink.tryEmitComplete();
373373
errorSink.tryEmitComplete();
374-
}).then(Mono.defer(() -> {
374+
375375
// Destroy process FIRST - this closes streams and unblocks readLine()
376-
logger.debug("Sending TERM to process");
376+
// Use blocking waitFor() instead of Mono.fromFuture(process.onExit())
377+
// to avoid ForkJoinPool.commonPool (per BEST-PRACTICES-REACTIVE-SCHEDULERS.md Rule 1)
377378
if (this.process != null) {
379+
logger.debug("Sending TERM to process");
378380
this.process.destroy();
379-
return Mono.fromFuture(process.onExit());
380-
}
381-
else {
382-
logger.warn("Process not started");
383-
return Mono.<Process>empty();
384-
}
385-
})).doOnNext(process -> {
386-
int exitCode = process.exitValue();
387-
// 143 = SIGTERM (128+15), 137 = SIGKILL (128+9) - expected when we destroy
388-
if (exitCode == 0 || exitCode == 143 || exitCode == 137) {
389-
logger.info("ACP agent process stopped (exit code {})", exitCode);
390-
}
391-
else {
392-
logger.warn("Process terminated unexpectedly with code {}", exitCode);
381+
try {
382+
boolean exited = process.waitFor(5, java.util.concurrent.TimeUnit.SECONDS);
383+
if (exited) {
384+
int exitCode = process.exitValue();
385+
// 143 = SIGTERM (128+15), 137 = SIGKILL (128+9) - expected when we destroy
386+
if (exitCode == 0 || exitCode == 143 || exitCode == 137) {
387+
logger.info("ACP agent process stopped (exit code {})", exitCode);
388+
}
389+
else {
390+
logger.warn("Process terminated unexpectedly with code {}", exitCode);
391+
}
392+
}
393+
else {
394+
logger.warn("Process did not exit within timeout, forcing kill");
395+
process.destroyForcibly();
396+
}
397+
}
398+
catch (InterruptedException e) {
399+
Thread.currentThread().interrupt();
400+
logger.debug("Interrupted while waiting for process exit");
401+
}
393402
}
394-
}).then(Mono.fromRunnable(() -> {
403+
404+
// Now that process is dead and streams closed, threads should be unblocked
395405
try {
396-
// Now that process is dead and streams closed, threads should be unblocked
397406
inboundScheduler.dispose();
398407
errorScheduler.dispose();
399408
outboundScheduler.dispose();
400-
401409
logger.debug("Graceful shutdown completed");
402410
}
403411
catch (Exception e) {
404412
logger.error("Error during graceful shutdown", e);
405413
}
406-
})).then();
414+
});
407415
}
408416

409417
public Sinks.Many<String> getErrorSink() {

0 commit comments

Comments
 (0)