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 a race condition of not getting all the output in .NET Image #229

Merged
merged 3 commits into from
Oct 5, 2020

Conversation

mikhailshilkov
Copy link
Member

Fix #200

There is a race condition in the .NET SDK. As observed in a local test, Exited event may fire before the last OutputDataReceived call. If that happens, the last chunk of the output won't be appended to stdout. The inspect command returns a single chunk of output. If it's missing, the output is empty, and the exception "No digest available for image" is fired.

One possible fix would be to wait for a call with e.Data equal to null which signals the completion of the output.

A simpler solution is suggested here and implemented in this PR: make and extra WaitForExit call in the ProcessExited handler. This flushes the buffers and lets the output sync in. My local tests confirmed the improvement: I don't get the issue anymore.

Repro program that I used: https://gist.github.com/mikhailshilkov/978ae50e26c6e712c56fbd4d73b48222

In addition, I moved Exited event handler registration to before the Start() call to avoid a potential miss. (I believe this is not related to this issue, though.)

cc @Aaronontheweb @dbeattie71

Copy link
Member

@justinvp justinvp left a comment

Choose a reason for hiding this comment

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

LGTM

@mikhailshilkov mikhailshilkov merged commit 01dfbf5 into master Oct 5, 2020
@pulumi-bot pulumi-bot deleted the mikhailshilkov/dotnet-output-race branch October 5, 2020 05:51
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