Re: [PATCH] receive-pack: interrupt pre-receive when client disconnects
From: Jiang Xin <hidden>
Date: 2022-01-26 07:17:57
On Wed, Jan 26, 2022 at 12:09 AM Robin Jarry [off-list ref] wrote:
When hitting ctrl-c on the client while a remote pre-receive hook is
running, receive-pack is not killed by SIGPIPE because the signal is
ignored. This is a side effect of commit ec7dbd145bd8 ("receive-pack:
allow hooks to ignore its standard input stream").
The pre-receive hook itself is not interrupted and does not receive any
error since its stdout is a pipe which is read in an async thread and
output back to the client socket in a side band channel.
After the pre-receive has exited the SIGPIPE default handler is restored
and if the hook did not report any error, objects are migrated from
temporary to permanent storage.We used to ignore the SIGPIPE signal when calling "pre-receive" hook, so we could tolerant a buggy "pre-receive" implementation which didn't consume all the input from "receive-pack". On the other side, "ctrl-c" from the client side will terminate "receive-pack", only if we do not ignore the SIGPIPE signal when running "pre-receive". Wouldn't this be much simpler: add a new configuration variable "receive.loosePreReceiveImplementation", and only ignore SIGPIPE when "receive-pack" turns off the config variable?
quoted hunk ↗ jump to hunk
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9f4a0b816cf9..0f41fe8c6a85 100644@@ -522,10 +540,24 @@ static int copy_to_sideband(int in, int out, void *arg) * Either we're not looking for a NUL signal, or we didn't see * it yet; just pass along the data. */ - send_sideband(1, 2, data, sz, use_sideband); + if (proc && proc->pid > 0) { + if (send_sideband2(1, 2, data, sz, use_sideband) < 0) + goto error; + } else + send_sideband(1, 2, data, sz, use_sideband); } close(in); return 0; +error: + close(in); + if (proc && proc->pid > 0) { + /* + * SIGPIPE would be more relevant but we want to make sure that + * the hook does not ignore the signal. + */ + kill(proc->pid, SIGKILL); + } + return -1; }
Kill the "pre-receive" process, so the calling of "finish_command(&proc)" at the end of "run_and_feed_hook()" will terminate "receive-pack".
quoted hunk ↗ jump to hunk
diff --git a/sideband.c b/sideband.c index 85bddfdcd4f5..27f8d653eb24 100644 --- a/sideband.c +++ b/sideband.c +static int send_sideband_priv(int fd, int band, const char *data, ssize_t sz, + int packet_max, int ignore_errors) { const char *p = data;@@ -265,13 +279,24 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma if (0 <= band) { xsnprintf(hdr, sizeof(hdr), "%04x", n + 5); hdr[4] = band; - write_or_die(fd, hdr, 5); + if (ignore_errors)
"ignore_errors" or "die_on_errors"?
+ write_or_die(fd, hdr, 5); + else if (write_in_full(fd, hdr, 5) < 0) + return -1;
-- Jiang Xin