Skip to content

Commit 19d5a18

Browse files
carldebillycarldebilly
authored
Merge pull request #22983 from unoplatform/dev/cdb/fix-git-zombie-processes
fix: Prevent git.exe zombie processes in MCP DevServer (#22982)
2 parents a88884c + 34f872e commit 19d5a18

3 files changed

Lines changed: 149 additions & 21 deletions

File tree

‎src/Uno.UI.DevServer.Cli/Helpers/SolutionFileFinder.cs‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ internal static class SolutionFileFinder
4141
private static readonly string[] HardcodedSkipDirs =
4242
["node_modules", "bin", "obj", ".vs", ".idea", "packages"];
4343

44+
/// <summary>
45+
/// Cached result of the last <c>git check-ignore</c> call. The cache key is the
46+
/// sorted set of paths that were checked; the value is the set of ignored paths.
47+
/// This avoids re-spawning git every 10 seconds when the input hasn't changed.
48+
/// The cache expires after <see cref="GitIgnoreCacheTtl"/> to pick up .gitignore changes.
49+
/// </summary>
50+
private static string? _cachedGitIgnoreCacheKey;
51+
private static HashSet<string>? _cachedGitIgnoreResult;
52+
private static DateTime _cachedGitIgnoreTimestamp;
53+
private static readonly TimeSpan GitIgnoreCacheTtl = TimeSpan.FromSeconds(60);
54+
4455
private static bool ShouldAlwaysSkipDirectory(string directoryName)
4556
{
4657
if (HardcodedSkipDirs.Contains(directoryName, StringComparer.OrdinalIgnoreCase))
@@ -114,6 +125,16 @@ .. results.Where(solution => !gitIgnoredSolutions.Contains(solution))
114125
return new HashSet<string>(StringComparer.OrdinalIgnoreCase);
115126
}
116127

128+
// Build a cache key from the sorted paths + git root so we don't
129+
// re-spawn git check-ignore every 10 seconds for the same input.
130+
var cacheKey = gitRoot + "|" + string.Join("|", paths.OrderBy(p => p, StringComparer.OrdinalIgnoreCase));
131+
if (cacheKey == _cachedGitIgnoreCacheKey
132+
&& _cachedGitIgnoreResult is not null
133+
&& DateTime.UtcNow - _cachedGitIgnoreTimestamp < GitIgnoreCacheTtl)
134+
{
135+
return _cachedGitIgnoreResult;
136+
}
137+
117138
try
118139
{
119140
// Build relative paths for git check-ignore (absolute Windows paths don't work)
@@ -128,33 +149,28 @@ .. results.Where(solution => !gitIgnoredSolutions.Contains(solution))
128149
}
129150

130151
// Pass paths as arguments (--stdin has issues on Windows via ProcessStartInfo)
131-
var psi = new ProcessStartInfo("git")
132-
{
133-
WorkingDirectory = gitRoot,
134-
RedirectStandardOutput = true,
135-
RedirectStandardError = true,
136-
UseShellExecute = false,
137-
CreateNoWindow = true,
138-
};
139-
psi.ArgumentList.Add("check-ignore");
140-
foreach (var relativePath in relativePaths)
141-
{
142-
psi.ArgumentList.Add(relativePath);
143-
}
152+
var psi = CreateGitCheckIgnoreStartInfo(gitRoot, relativePaths);
144153

145154
using var process = Process.Start(psi);
146155
if (process is null)
147156
{
148157
return null; // git not available
149158
}
150159

151-
// Kill the process after a timeout to prevent hanging. This
152-
// ensures that the subsequent synchronous ReadToEnd() will
153-
// return promptly because killing closes the stdout pipe.
160+
// Close stdin immediately — git check-ignore reads from args, not stdin.
161+
// This signals EOF on the redirected pipe so git doesn't wait for input.
162+
process.StandardInput.Close();
163+
164+
// Drain stderr on a background thread to prevent pipe buffer deadlocks.
165+
// The OS pipe buffer is ~4 KB; if git writes enough to stderr while we
166+
// block on WaitForExit reading only stdout, both processes deadlock.
167+
process.ErrorDataReceived += (_, _) => { };
168+
process.BeginErrorReadLine();
169+
154170
if (!process.WaitForExit(2000))
155171
{
156-
// git hung — kill it and fall back
157-
try { process.Kill(); } catch { /* best effort */ }
172+
// git hung — kill the entire process tree and fall back
173+
try { process.Kill(entireProcessTree: true); } catch { /* best effort */ }
158174
return null;
159175
}
160176

@@ -177,7 +193,10 @@ .. results.Where(solution => !gitIgnoredSolutions.Contains(solution))
177193
}
178194
}
179195

180-
return ignored;
196+
_cachedGitIgnoreCacheKey = cacheKey;
197+
_cachedGitIgnoreResult = new HashSet<string>(ignored, StringComparer.OrdinalIgnoreCase);
198+
_cachedGitIgnoreTimestamp = DateTime.UtcNow;
199+
return _cachedGitIgnoreResult;
181200
}
182201
catch
183202
{
@@ -238,4 +257,28 @@ private static void SearchDirectory(string directory, int currentDepth, int maxD
238257
// Skip directories we can't access or that disappeared during scanning
239258
}
240259
}
260+
261+
/// <summary>
262+
/// Creates the <see cref="ProcessStartInfo"/> for <c>git check-ignore</c>.
263+
/// Exposed as <c>internal</c> so tests can verify critical properties
264+
/// (e.g. <see cref="ProcessStartInfo.RedirectStandardInput"/>).
265+
/// </summary>
266+
internal static ProcessStartInfo CreateGitCheckIgnoreStartInfo(string gitRoot, List<string> relativePaths)
267+
{
268+
var psi = new ProcessStartInfo("git")
269+
{
270+
WorkingDirectory = gitRoot,
271+
RedirectStandardOutput = true,
272+
RedirectStandardError = true,
273+
RedirectStandardInput = true, // Prevent inheriting parent's stdin (e.g. libuv named pipe from MCP client)
274+
UseShellExecute = false,
275+
CreateNoWindow = true,
276+
};
277+
psi.ArgumentList.Add("check-ignore");
278+
foreach (var relativePath in relativePaths)
279+
{
280+
psi.ArgumentList.Add(relativePath);
281+
}
282+
return psi;
283+
}
241284
}

‎src/Uno.UI.DevServer.Cli/Mcp/DevServerMonitor.cs‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ private void LogTimeline(string stage, long elapsedMilliseconds, string details)
5757
/// </summary>
5858
public IReadOnlyList<string> DiscoveredSolutions { get; private set; } = [];
5959

60+
/// <summary>
61+
/// Set to true when the monitor has determined that this workspace is not
62+
/// an Uno Platform project (no host found after <see cref="MaxNoHostRetries"/>
63+
/// discovery attempts). The monitor stops scanning to avoid wasting resources.
64+
/// </summary>
65+
public bool NotAnUnoWorkspace { get; private set; }
66+
67+
private int _noHostDiscoveryCount;
68+
private const int MaxNoHostRetries = 3;
69+
6070
internal void StartMonitoring(string currentDirectory, int port, List<string> forwardedArgs,
6171
WorkspaceResolution? workspaceResolution = null)
6272
{
@@ -69,6 +79,8 @@ internal void StartMonitoring(string currentDirectory, int port, List<string> fo
6979
_forwardedArgs = forwardedArgs;
7080
_currentDirectory = currentDirectory;
7181
_workspaceResolution = workspaceResolution;
82+
_noHostDiscoveryCount = 0;
83+
NotAnUnoWorkspace = false;
7284

7385
var forwardedArgsDisplay = string.Join(" ", _forwardedArgs);
7486
_logger.LogTrace(
@@ -156,11 +168,24 @@ private async Task RunMonitor(CancellationToken ct)
156168

157169
if (hostPath is null)
158170
{
159-
_logger.LogTrace("DevServerMonitor could not resolve a host executable in {Directory}",
160-
_currentDirectory);
171+
_noHostDiscoveryCount++;
172+
_logger.LogTrace(
173+
"DevServerMonitor could not resolve a host executable in {Directory} (attempt {Count})",
174+
_currentDirectory, _noHostDiscoveryCount);
175+
176+
if (_noHostDiscoveryCount >= MaxNoHostRetries)
177+
{
178+
_logger.LogInformation(
179+
"No Uno SDK host found after {Count} attempts in {Directory} — stopping monitor. "
180+
+ "This workspace does not appear to be an Uno Platform project.",
181+
_noHostDiscoveryCount, _currentDirectory);
182+
NotAnUnoWorkspace = true;
183+
break;
184+
}
161185
}
162186
else
163187
{
188+
_noHostDiscoveryCount = 0; // Reset on success
164189
_logger.LogTrace("DevServerMonitor resolved host executable {HostPath}", hostPath);
165190
}
166191

‎src/Uno.UI.RemoteControl.DevServer.Tests/Mcp/Given_DevServerMonitor.cs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,66 @@ public void GetGitIgnoredPaths_ReturnsNull_WhenGitNotAvailable()
440440
}
441441
}
442442

443+
/// <summary>
444+
/// Regression test for unoplatform/uno#22982: git.exe zombie processes.
445+
/// When the DevServer runs as an MCP server behind a libuv-based client,
446+
/// git.exe inherits the parent's stdin (a libuv named pipe) and hangs.
447+
/// The fix is to set RedirectStandardInput = true in the ProcessStartInfo
448+
/// so git gets a fresh empty pipe instead of the inherited handle.
449+
/// </summary>
450+
[TestMethod]
451+
[Description("ProcessStartInfo for git check-ignore must redirect stdin to prevent libuv pipe inheritance (fixes #22982)")]
452+
public void GetGitIgnoredPaths_ProcessStartInfo_RedirectsStdin()
453+
{
454+
var psi = SolutionFileFinder.CreateGitCheckIgnoreStartInfo(".", ["some/path"]);
455+
456+
psi.RedirectStandardInput.Should().BeTrue(
457+
"git.exe must NOT inherit the parent's stdin — when stdin is a libuv named pipe "
458+
+ "(from an MCP client), git hangs during I/O initialization (see #22982)");
459+
psi.RedirectStandardOutput.Should().BeTrue();
460+
psi.RedirectStandardError.Should().BeTrue();
461+
psi.UseShellExecute.Should().BeFalse(
462+
"UseShellExecute=false is required for stream redirection");
463+
}
464+
465+
[TestMethod]
466+
[Description("Calling GetGitIgnoredPaths twice with the same input should not spawn git twice (#22982)")]
467+
public void GetGitIgnoredPaths_ReturnsCachedResult_WhenInputUnchanged()
468+
{
469+
var tempDir = Path.Combine(Path.GetTempPath(), $"cache-test-{Guid.NewGuid():N}");
470+
Directory.CreateDirectory(tempDir);
471+
try
472+
{
473+
RunGit(tempDir, "init");
474+
RunGit(tempDir, "config user.email test@test.com");
475+
RunGit(tempDir, "config user.name Test");
476+
File.WriteAllText(Path.Combine(tempDir, ".gitignore"), "ignored/\n");
477+
RunGit(tempDir, "add .gitignore");
478+
RunGit(tempDir, "commit -m init");
479+
480+
var ignoredDir = Path.Combine(tempDir, "ignored");
481+
var visibleDir = Path.Combine(tempDir, "src");
482+
Directory.CreateDirectory(ignoredDir);
483+
Directory.CreateDirectory(visibleDir);
484+
485+
var paths = new List<string> { ignoredDir, visibleDir };
486+
487+
var result1 = SolutionFileFinder.GetGitIgnoredPaths(paths, tempDir);
488+
var result2 = SolutionFileFinder.GetGitIgnoredPaths(paths, tempDir);
489+
490+
result1.Should().NotBeNull();
491+
result2.Should().NotBeNull();
492+
result1!.Should().Contain(ignoredDir);
493+
// The second call should return the same cached result
494+
result2.Should().BeEquivalentTo(result1,
495+
"the second call with identical input should return the cached result without spawning git again");
496+
}
497+
finally
498+
{
499+
ForceDeleteDirectory(tempDir);
500+
}
501+
}
502+
443503
[TestMethod]
444504
public void FindSolutionFiles_UsesHardcodedSkipList_WhenGitFails()
445505
{

0 commit comments

Comments
 (0)