Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"git-credential-manager-core.exe diagnose" hangs when "git config --list" output is long #795

Closed
wm1 opened this issue Jul 18, 2022 · 1 comment · Fixed by #804
Closed
Labels
bug A bug in Git Credential Manager

Comments

@wm1
Copy link

wm1 commented Jul 18, 2022

Repro:

  • In a blank folder, git init and then modify .git\config to make the file bigger than 4K
  • Run diagnose sub-command, it will hang at waiting for git config --list output.

[ OK ] Environment
[ OK ] File system
[ OK ] Networking
[ xxx ] Git

The underline issue is that GCM spawns a child process and redirects child's stdout. There is a size limit for the pipe buffer, and if the parent process does not read data out quickly, the child process will stall at writing to stdout.

A fix would be to add Process.OutputDataReceived to GitDiagnostic.RunInternalAsync.

I'd suggest reviewing all places that spawn child process for similar issue.

@wm1
Copy link
Author

wm1 commented Jul 18, 2022

diff --git a/src/shared/Core/Diagnostics/GitDiagnostic.cs b/src/shared/Core/Diagnostics/GitDiagnostic.cs
index fe6a326..53486e2 100644
--- a/src/shared/Core/Diagnostics/GitDiagnostic.cs
+++ b/src/shared/Core/Diagnostics/GitDiagnostic.cs
@@ -8,6 +8,7 @@ namespace GitCredentialManager.Diagnostics
     public class GitDiagnostic : Diagnostic
     {
         private readonly IGit _git;
+        private StringBuilder _output = null;

         public GitDiagnostic(IGit git)
             : base("Git")
@@ -15,6 +16,7 @@ namespace GitCredentialManager.Diagnostics
             EnsureArgument.NotNull(git, nameof(git));

             _git = git;
+            _output = new StringBuilder();
         }

         protected override Task<bool> RunInternalAsync(StringBuilder log, IList<string> additionalFiles)
@@ -31,9 +33,20 @@ namespace GitCredentialManager.Diagnostics

             log.Append("Listing all Git configuration...");
             Process configProc = _git.CreateProcess("config --list --show-origin");
+
+            configProc.OutputDataReceived += new DataReceivedEventHandler((sender, e) =>
+            {
+                _output.Append(e.Data + "\n");
+            });
+
             configProc.Start();
+
+            // Asynchronously read the standard output of the spawned process.
+            // This raises OutputDataReceived events for each line of output.
+            configProc.BeginOutputReadLine();
+
             configProc.WaitForExit();
-            string gitConfig = configProc.StandardOutput.ReadToEnd().TrimEnd();
+            string gitConfig = _output.ToString();
             log.AppendLine(" OK");
             log.AppendLine("Git configuration:");
             log.AppendLine(gitConfig);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug in Git Credential Manager
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants