Re: [PATCHv3] gpg-interface: check gpg signature creation status

[Jeff King <peff@peff.net>]

On Tue, Jun 14, 2016 at 06:26:33PM -0400, Jeff King wrote:

> > > >  	bottom = signature->len;
> > > > -	len = strbuf_read(signature, gpg.out, 1024);
> > > > +	strbuf_read(signature, gpg.out, 1024);
> > > > +	strbuf_read(&err, gpg.err, 0);
> > > 
> > > Hmmmm, isn't this asking for a deadlock?  When GPG spews more than
> > > what would fit in a pipe buffer to its standard error (hence gets
> > > blocked), its standard output may not complete, and the we would get
> > > stuck by attempting to read from gpg.out, failing to reach the other
> > > strbuf_read() that would unblock GPG by reading from gpg.err?
> > 
> > Yeah, it definitely is a deadlock. I think we'd need a select loop to
> > read into multiple strbufs at once (we can't use "struct async" because
> > that might happen in another process).
> 
> Something like this on top of Michael's patch (only lightly tested).

I wondered if this is something we might run into in other places, but
just haven't in practice. After grepping existing calls to strbuf_read(),
I think the only other case is the one in verify_signed_buffer(), which
reads all of stderr before trying to read stdout. I suspect that _could_
deadlock but doesn't tend to in practice.

Interestingly, it also writes in full to gpg's stdin before reading
anything. If gpg buffers all of its input before writing, that's fine,
but in theory it does not have to. I have no idea if it's possible for
it to be a problem in practice.

That can be solved in the same select loop, though obviously it's not
just strbuf_read_parallel() at that point, but rather a kind of a
feed_and_capture_command().

-Peff