Skip to content

Conversation

@jaredpar
Copy link
Member

This cleans up how we handle output capture on .NET Core. This moves from a lot of mutable static static, that would break if tests ran in parallel, to instance state that uses locks to guard againtst concurrent runs.

This cleans up how we handle output capture on .NET Core. This moves from a lot of mutable `static` static, that would break if tests ran in parallel, to instance state that uses locks to guard againtst concurrent runs.
@jaredpar jaredpar requested review from a team as code owners April 18, 2025 18:21
@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 18, 2025
@jaredpar
Copy link
Member Author

@dotnet/roslyn-compiler PTAL

@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 19, 2025
public interface IRuntimeEnvironment : IDisposable
{
(int ExitCode, string Output, string ErrorOutput) Execute(string[] args, int? maxOutputLength);
(int ExitCode, string Output, string ErrorOutput) Execute(string[] args);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the maxOutputLength parameter was used to guard againts infinite loops, seemed useful but it's removed now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was only done on .NET Core though, not .NET Framework. I considered putting the same code path in .NET Framework but that guard hasn't gotten tripped anytime that I remember. Removing the guard means we will still get an exception in an infinite loop, just it will be an OOM exception. So decided to simplify here and remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, I've seen that guard hit locally during development, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants