On Wed, Feb 22, 2012 at 02:13, Jeff King <peff@xxxxxxxx> wrote:
> On Fri, Jan 20, 2012 at 09:03:31AM -0800, Shawn O. Pearce wrote:
> This hunk is causing intermittent failures of t5541 for me, especially
> when the system is under heavy load (e.g., make -j32 test).
...
> @@ -220,15 +221,21 @@ static struct child_process *get_helper(struct transport *transport)
> static int disconnect_helper(struct transport *transport)
> {
> struct helper_data *data = transport->data;
> - struct strbuf buf = STRBUF_INIT;
> int res = 0;
>
> if (data->helper) {
> if (debug)
> fprintf(stderr, "Debug: Disconnecting.\n");
> if (!data->no_disconnect_req) {
> - strbuf_addf(&buf, "\n");
> - sendline(data, &buf);
> + /*
> + * Ignore write errors; there's nothing we can do,
> + * since we're about to close the pipe anyway. And the
> + * most likely error is EPIPE due to the helper dying
> + * to report an error itself.
> + */
> + sigchain_push(SIGPIPE, SIG_IGN);
> + xwrite(data->helper->in, "\n", 1);
> + sigchain_pop(SIGPIPE);
> }
> close(data->helper->in);
> close(data->helper->out);
>
> which makes the t5541 failures go away for me. What do you think?
This sounds right to me. Its unfortunate that we missed the error
status output when we built the remote helper protocol, but your patch
above might be the best we can do now.
Eh, well, actually we could have the helper advertise a new capability
that can be enabled to return exit status. That is a much bigger
change, and even if we do it for remote-curl (since that is in tree
and easy to update) we still need your patch for the same race
condition for out of tree helpers (which Google actually has so I care
about out of tree helpers too).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html