Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,11 @@ protected void handleOutbound(Function<Flux<JSONRPCMessage>, Flux<JSONRPCMessage
}

/**
* Gracefully closes the transport by destroying the process and disposing of the
* schedulers. This method sends a TERM signal to the process and waits for it to exit
* before cleaning up resources.
* Gracefully closes the transport by destroying the server process tree and disposing
* of the schedulers. This method sends a TERM signal to every process in the tree
* (descendants first, then the root) and waits for the root to exit before cleaning
* up resources. Tree-aware termination prevents leaked child processes when a wrapper
* command such as {@code npx} or {@code cmd.exe /c npx.cmd} is used.
* @return A Mono that completes when the transport is closed
*/
@Override
Expand All @@ -346,9 +348,9 @@ public Mono<Void> closeGracefully() {
// Give a short time for any pending messages to be processed
return Mono.delay(Duration.ofMillis(100)).then();
})).then(Mono.defer(() -> {
logger.debug("Sending TERM to process");
logger.debug("Sending TERM to process tree");
if (this.process != null) {
this.process.destroy();
destroyProcessTree(this.process);
return Mono.fromFuture(process.onExit());
}
else {
Expand Down Expand Up @@ -387,4 +389,21 @@ public <T> T unmarshalFrom(Object data, TypeRef<T> typeRef) {
return this.jsonMapper.convertValue(data, typeRef);
}

/**
* Destroys {@code process} together with any descendant processes spawned beneath it.
* Wrapper invocations such as {@code cmd.exe /c npx.cmd ...} on Windows or
* {@code npx} on POSIX systems spawn additional child processes; destroying only the
* wrapper leaves those descendants orphaned and prevents graceful JVM shutdown.
* <p>
* The descendant snapshot from {@link ProcessHandle#descendants()} is best-effort:
* processes that fork concurrently with this call may not be signalled. The wider
* tree typically dies anyway once its parent is gone.
* <p>
* Visible for tests.
*/
static void destroyProcessTree(Process process) {
process.descendants().forEach(ProcessHandle::destroy);
process.destroy();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright 2026-2026 the original author or authors.
*/
package io.modelcontextprotocol.client.transport;

import java.time.Duration;
import java.util.List;
import java.util.stream.Collectors;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.awaitility.Awaitility.await;

/**
* Tests for {@link StdioClientTransport#destroyProcessTree(Process)}.
*
* <p>
* Regression coverage for the case where {@code closeGracefully()} previously left
* orphaned descendant processes (e.g. {@code node} spawned beneath {@code npx} on
* Windows). The tree-aware destroy must terminate the entire descendant chain, not just
* the root.
*/
class StdioClientTransportDestroyTreeTests {

@Test
@DisabledOnOs(OS.WINDOWS) // sh + & + wait is POSIX; CI is ubuntu-latest
void destroyProcessTreeTerminatesRootAndDescendants() throws Exception {
// `sh` forks two long-sleeping children. Killing only `sh` would leave both
// children alive — exactly the bug reported in #496 (but reproduced portably).
Process root = new ProcessBuilder("sh", "-c", "sleep 60 & sleep 60 & wait").start();

await().atMost(Duration.ofSeconds(3)).until(() -> root.descendants().count() >= 2);
List<ProcessHandle> descendants = root.descendants().collect(Collectors.toList());
assertThat(descendants).hasSizeGreaterThanOrEqualTo(2);

StdioClientTransport.destroyProcessTree(root);

await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> {
assertThat(root.isAlive()).as("root sh process").isFalse();
for (ProcessHandle d : descendants) {
assertThat(d.isAlive()).as("descendant pid=%s", d.pid()).isFalse();
}
});
}

@Test
@DisabledOnOs(OS.WINDOWS)
void destroyProcessTreeTerminatesRootWithNoDescendants() throws Exception {
Process root = new ProcessBuilder("sleep", "60").start();

assertThat(root.isAlive()).isTrue();

StdioClientTransport.destroyProcessTree(root);

await().atMost(Duration.ofSeconds(5)).until(() -> !root.isAlive());
}

@Test
void destroyProcessTreeIsSafeOnAlreadyTerminatedProcess() throws Exception {
// Pick a command that exists on every supported OS and exits immediately.
String[] cmd = System.getProperty("os.name").toLowerCase().contains("win")
? new String[] { "cmd.exe", "/c", "exit", "0" } : new String[] { "sh", "-c", "exit 0" };
Process root = new ProcessBuilder(cmd).start();
root.waitFor();
assertThat(root.isAlive()).isFalse();

assertThatCode(() -> StdioClientTransport.destroyProcessTree(root)).doesNotThrowAnyException();
}

}