Re: [RFC PATCH] vreportf: ensure sensible ordering of normal and error output

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Nov 30, 2021 at 2:14 AM Jeff King <peff@peff.net> wrote:
>   - 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.