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

make in-process program's output deterministic #7

Merged
merged 1 commit into from Mar 17, 2020

Conversation

@gbrlsnchs
Copy link
Contributor

gbrlsnchs commented Mar 17, 2020

When an in-process program writes to both stdout and stderr, one at a
time, sequentially, the output is non-deterministic. More specifically,
the order of writing might not be respected.

This happens because there is one pipe for each file, and each reading
pipe is fetched concurrently (each in its own goroutine), in order to be
subsequently copied to a locking writer.

Since there is a goroutine for each reading pipe to be copied to the
locking writer, a pipe that was written later might end up copied before
a pipe that was written earlier.

The locking writer does prevent race condition between writes but
doesn't solve the order problem. The solution, then, is to have a single
pipe for both files, allowing sequential writing to be guaranteed while
also unifying both outputs deterministically in a single buffer.

When an in-process program writes to both stdout and stderr, one at a
time, sequentially, the output is non-deterministic. More specifically,
the order of writing might not be respected.

This happens because there is one pipe for each file, and each reading
pipe is fetched concurrently (each in its own goroutine), in order to be
subsequently copied to a locking writer.

Since there is a goroutine for each reading pipe to be copied to the
locking writer, a pipe that was written later might end up copied before
a pipe that was written earlier.

The locking writer does prevent race condition between writes but
doesn't solve the order problem. The solution, then, is to have a single
pipe for both files, allowing sequential writing to be guaranteed while
also unifying both outputs deterministically in a single buffer.
@vangent vangent requested review from jba and vangent Mar 17, 2020
@vangent

This comment has been minimized.

Copy link
Collaborator

vangent commented Mar 17, 2020

@jba it looks good to me, I'll let you merge,

@jba
jba approved these changes Mar 17, 2020
@jba jba merged commit 2cfe900 into google:master Mar 17, 2020
2 checks passed
2 checks passed
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vangent

This comment has been minimized.

Copy link
Collaborator

vangent commented Mar 17, 2020

@gbrlsnchs thanks for the contribution! Your clear and detailed PR description was great and helped understand the problem & solution.

@gbrlsnchs gbrlsnchs deleted the gbrlsnchs:fix-pipe-race branch Mar 17, 2020
@gbrlsnchs

This comment has been minimized.

Copy link
Contributor Author

gbrlsnchs commented Mar 17, 2020

I'm glad I could help. Thank you for your reviews and this useful lib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.