From 796d7d46b69cb32723cdeecfe9dcf55f0b5f55e7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 17:33:30 +0000 Subject: [PATCH 1/6] Initial plan From 50263cf6fe54da959d05414df27aedf2c465a274 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 17:45:24 +0000 Subject: [PATCH 2/6] Fix race condition in DisposeProcess that could skip flushing stderr events When GetUnexpectedExitExceptionAsync sees HasExited as false and returns null, DisposeProcess is called next. By that time the process may have exited, causing it to skip KillTree and call Dispose() without first calling WaitForExit() to flush ErrorDataReceived callbacks. Add a WaitForExit() call for already-exited processes to ensure all redirected output events are dispatched before disposal. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Client/StdioClientTransport.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index 95f5221f2..a0c14e439 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -227,6 +227,13 @@ internal static void DisposeProcess( // and Node.js does not kill its children when it exits properly. process.KillTree(shutdownTimeout); } + else if (HasExited(process)) + { + // The process has already exited. Call WaitForExit() (no arguments) + // to ensure all redirected stderr/stdout events have been dispatched + // before disposing. + process.WaitForExit(); + } } finally { From 0a6158f57bd5e8991cc9112b3769bce84744fa36 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 18:33:30 +0000 Subject: [PATCH 3/6] Remove redundant HasExited check in DisposeProcess else branch After line 222 recomputes processRunning as `processRunning && !HasExited(process)`, the else branch already implies HasExited was true (when processRunning was originally true). The redundant check is unnecessary. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- src/ModelContextProtocol.Core/Client/StdioClientTransport.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index a0c14e439..580e0e49c 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -227,7 +227,7 @@ internal static void DisposeProcess( // and Node.js does not kill its children when it exits properly. process.KillTree(shutdownTimeout); } - else if (HasExited(process)) + else { // The process has already exited. Call WaitForExit() (no arguments) // to ensure all redirected stderr/stdout events have been dispatched From 1a58fd517e122e90cc48076a906de512b04fd4f0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 18:38:35 +0000 Subject: [PATCH 4/6] Always call WaitForExit() (no args) before Dispose() in DisposeProcess The previous fix only called WaitForExit() in the else branch (process already exited). But when the process is still running, KillTree is called which uses either Kill+WaitForExit(int) or returns early if the process already exited - neither of which guarantees ErrorDataReceived event dispatch. Move WaitForExit() (no args) to be unconditional after the if/else block so it always runs before Dispose(). Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Client/StdioClientTransport.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index 580e0e49c..fd4a1b8e0 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -227,13 +227,11 @@ internal static void DisposeProcess( // and Node.js does not kill its children when it exits properly. process.KillTree(shutdownTimeout); } - else - { - // The process has already exited. Call WaitForExit() (no arguments) - // to ensure all redirected stderr/stdout events have been dispatched - // before disposing. - process.WaitForExit(); - } + + // Ensure all redirected stderr/stdout events have been dispatched + // before disposing. Only the no-arg WaitForExit() guarantees this; + // WaitForExit(int) (used by KillTree) does not. + process.WaitForExit(); } finally { From 1826e6bcf15f68922d12e15381a9bd0742bfac46 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 19:04:01 +0000 Subject: [PATCH 5/6] Guard WaitForExit() with HasExited check to avoid hanging if kill times out If KillTree's kill succeeds but WaitForExit(int) times out, the process is still alive. Calling WaitForExit() (no args) unconditionally would then block indefinitely. Guard with HasExited to only flush stderr events when we know the process has actually exited. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Client/StdioClientTransport.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index fd4a1b8e0..67ccb6470 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -230,8 +230,12 @@ internal static void DisposeProcess( // Ensure all redirected stderr/stdout events have been dispatched // before disposing. Only the no-arg WaitForExit() guarantees this; - // WaitForExit(int) (used by KillTree) does not. - process.WaitForExit(); + // WaitForExit(int) (used by KillTree) does not. Guard with HasExited + // to avoid hanging indefinitely if the process couldn't be killed. + if (HasExited(process)) + { + process.WaitForExit(); + } } finally { From 891c34c51d913b66db64e92dd6a26d59553a98e5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 19:51:24 +0000 Subject: [PATCH 6/6] Only call WaitForExit() when process exited on its own, not after KillTree The previous fix called WaitForExit() unconditionally or guarded only by HasExited, which could hang on Windows when KillTree left child processes holding output handles. Now WaitForExit() is only called when the process exited before DisposeProcess was called (!processRunning), meaning KillTree was not invoked. In that case, no child processes hold handles and WaitForExit() completes quickly. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Client/StdioClientTransport.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index 67ccb6470..191a512b1 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -230,9 +230,12 @@ internal static void DisposeProcess( // Ensure all redirected stderr/stdout events have been dispatched // before disposing. Only the no-arg WaitForExit() guarantees this; - // WaitForExit(int) (used by KillTree) does not. Guard with HasExited - // to avoid hanging indefinitely if the process couldn't be killed. - if (HasExited(process)) + // WaitForExit(int) (as used by KillTree) does not. + // This should not hang: either the process already exited on its own + // (no child processes holding handles), or KillTree killed the entire + // process tree. If it does take too long, the test infrastructure's + // own timeout will catch it. + if (!processRunning && HasExited(process)) { process.WaitForExit(); }