Skip to content
This repository was archived by the owner on Nov 20, 2023. It is now read-only.

DockerRunner: include stderr in log on failure.#1059

Merged
jkotalik merged 1 commit into
dotnet:mainfrom
tmds:docker_include_stderr
May 25, 2021
Merged

DockerRunner: include stderr in log on failure.#1059
jkotalik merged 1 commit into
dotnet:mainfrom
tmds:docker_include_stderr

Conversation

@tmds

@tmds tmds commented May 25, 2021

Copy link
Copy Markdown
Member

@jkotalik ptal

// Even though the Exited event has been raised, WaitForExit() must still be called to ensure the output buffers
// have been flushed before the process is considered completely done.
process.WaitForExit();
lock (process)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I noticed this potential race so I added a lock for it.

cancellationToken: cancellationToken,
outputDataReceived: data => service.Logs.OnNext($"[{replica}]: {data}"),
errorDataReceived: data => service.Logs.OnNext($"[{replica}]: {data}"));
errorDataReceived: data => { service.Logs.OnNext($"[{replica}]: {data}"); stderr.AppendLine(data); });

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the actual fix.

@jkotalik jkotalik merged commit b1aaca8 into dotnet:main May 25, 2021
@jkotalik

Copy link
Copy Markdown
Contributor

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants