x/playground: interleaving stdout and stderr is broken #24659
Comments
|
(CC: @andybons @josharian) |
bcmills
added
the
NeedsFix
|
Change https://golang.org/cl/104857 mentions this issue: |
|
Change https://golang.org/cl/105235 mentions this issue: |
|
Here is some debug output collected by using this: Running this program gives following recombinations of output: I have applied the strict time patch to the go and built playground, got following: We do get increments in header for every write operation. It is interesting to note that header and actual data are not passed as a single piece to @bcmills, does it make sense? Update: I am working on the playground change complementing the fix by Bryan. |
|
Nice analysis. Separate buffering sounds right. (And if we buffer the two streams separately, we can do an O(n) sorted merge instead of an O(n log n) sort!) |
|
Change https://golang.org/cl/105496 mentions this issue: |
|
Change https://golang.org/cl/106216 mentions this issue: |
The process reading the output of the binary may read stderr and stdout separately, and may interleave reads from the two streams arbitrarily. Because we explicitly serialize writes on the writer side, we can reuse a timestamp within a single stream without losing information; however, if we use the same timestamp for write on both streams, the reader can't tell how to interleave them. This change ensures that every time we change between the two fds, we also bump the timestamp. That way, writes within a stream continue to show the same timestamp, but a sorted merge of the contents of the two streams always interleaves them in the correct order. This still requires a corresponding change to the Playground parser to actually reconstruct the correct interleaving. It currently merges the two streams without reordering them; it should instead buffer them separately and perform a sorted merge. (See https://golang.org/cl/105496.) Updates #24615. Updates #24659. Change-Id: Id789dfcc02eb4247906c9ddad38dac50cf829979 Reviewed-on: https://go-review.googlesource.com/105235 Run-TryBot: Bryan C. Mills <bcmills@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Yury Smolsky <yury@smolsky.by> Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
Change https://golang.org/cl/106475 mentions this issue: |
Add a test case (currently flaky) that checks for deterministic output ordering. Updates golang/go#24615. Updates golang/go#24659. Change-Id: I7edc5d77a890edcd684ddf2aeee4c7a7dea68af1 Reviewed-on: https://go-review.googlesource.com/106216 Reviewed-by: Andrew Bonventre <andybons@golang.org>
Add a test case that checks fake timestamps generated by the patched runtime.write. Updates golang/go#24615 Updates golang/go#24659 Change-Id: I210cfd4500e7f72567539dd9b3e23da4f907a3e2 Reviewed-on: https://go-review.googlesource.com/106475 Reviewed-by: Bryan C. Mills <bcmills@google.com>
The expected output of the program https://play.golang.org/p/5agzWi_tyrw is
Actual output is
I see a way to fix this in playground by writing both
stderrandstdoutstreams into one:stdout. Since playground does not displaystderrany different thanstdout, we can do this to make its behaviour consistent.This issue affects tests executed in playground. If the test outputs anything to
stderrthen the order of lines is not consistent. See #24615The text was updated successfully, but these errors were encountered: