Re: [RFC PATCH] vreportf: ensure sensible ordering of normal and error output
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Nov 30, 2021, 14:15 UTC
- Message-ID
- <CAPig+cT4+i-R74Lnt9RgkXkDVqx5nThqEMvKG05SP6iRSSsm2w@mail.gmail.com>
- In-Reply-To
- <YaXPUe9Sz3JBlzYL@coredump.intra.peff.net>
On Tue, Nov 30, 2021 at 2:14 AM Jeff King <peff@peff.net> wrote:
Show 15 quoted lines
> - flushing causes us to block. This implies our stdout is connected to > a pipe or socket, and the other side is not expecting to read. A > plausible case here is a client sending us a big input which we find > to be bogus (maybe index-pack checking an incoming pack). We call > die() to complain about the input, but the client is still writing. > In the current code, we'd write out our error and then exit; the > client would get SIGPIPE or a write() error and abort. But with a > flush here, we could block writing back to the client, and now we're > in a deadlock; they are trying to write to us but we are no longer > reading, and we are blocked trying to get out a little bit of > irrelevant stdout data. > > Of the two, the deadlock case worries me more, just because it would be > quiet subtle and racy. As I said, I think we may be OK, but my reasoning > there is pretty hand-wavy.
Flushing stdout only if it is attached to a terminal:
if (isatty(1))
fflush(stdout);should address this potential-deadlock concern, I think(?). It's rather ugly, though, and entering the realm of too-much-special-casing; it feels like it has the potential of heading down a rabbit hole where we find more cases which need to be handled specially.