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

Fix: Retrieving dotnet runtimes hangs when there are too many runtimes installed #119

Merged
merged 1 commit into from
Oct 23, 2021

Conversation

Booksbaum
Copy link
Contributor

Test Main tests.can get runtimes hangs on my PC.

Reason: ProjInfo calls dotnet --list-runtimes by executing dotnet, waiting for it to finish, and then reading the output:

let private execDotnet (cwd: DirectoryInfo) (binaryFullPath: FileInfo) args =
let info = ProcessStartInfo()
info.WorkingDirectory <- cwd.FullName
info.FileName <- binaryFullPath.FullName
for arg in args do
info.ArgumentList.Add arg
info.RedirectStandardOutput <- true
let p = System.Diagnostics.Process.Start(info)
p.WaitForExit()
seq {
while not p.StandardOutput.EndOfStream do
yield p.StandardOutput.ReadLine()
}

Important: First Waiting for exit, Then reading output

But: The executing Process blocks when its output buffer is full and not read (see Remarks for Process.StandardOutput).
-> When there are too many runtimes installed (-> too many lines from --list-runtimes), calling dotnet --list-runtimes hangs.

I installed at least one new dotnet version over the last days (-> several additional runtimes). Which seems to just crossed the line between "working" and "buffer full"
(there are more runtimes than sdks -> happend with --list-runtimes, not (yet) with --list-sdks)




Fix: Read lines while executing

@baronfel
Copy link
Collaborator

This makes good sense 👍 Thanks for this change.

@baronfel baronfel merged commit ce9f379 into ionide:master Oct 23, 2021
@Booksbaum Booksbaum deleted the dotnet-block branch October 24, 2021 07:46
slang25 referenced this pull request in jeffkl/MSBuildProjectCreator Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants