All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Jarry <robin@jarry.cc>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Nicolas Dichtel <nicolas.dichtel@6wind.com>,
	Jan Smets <jan.smets@nokia.com>,
	Stephen Morton <stephen.morton@nokia.com>,
	Jeff King <peff@peff.net>, Robin Jarry <robin@jarry.cc>
Subject: [PATCH v2] receive-pack: ignore SIGPIPE while reporting status to client
Date: Sat,  6 Nov 2021 23:03:59 +0100	[thread overview]
Message-ID: <20211106220358.144886-1-robin@jarry.cc> (raw)
In-Reply-To: <20211104133546.1967308-1-robin.jarry@6wind.com>

When a remote client exits while the 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, 2014-09-12).

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.

Before running the post-receive hook, status info is reported back to
the client. Since the client has died, receive-pack is killed by SIGPIPE
and post-receive is never executed.

The post-receive hook is often used to send email notifications (see
contrib/hooks/post-receive-email), update bug trackers, start automatic
builds, etc. Not executing it after an interrupted yet "successful" push
can lead to inconsistencies.

Ignore SIGPIPE before reporting status to the client to increase the
chances of post-receive running if pre-receive was successful. This does
not guarantee 100% consistency but it should resist early disconnection
by the client.

Signed-off-by: Robin Jarry <robin@jarry.cc>
---
Ideally, pre-receive should not be allowed to succeed if the client has
disconnected before objects have been migrated from temporary to
permanent storage. But that is another topic, and I think it would
complement this patch.

 builtin/receive-pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 49b846d96052..5fe7992028d4 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2564,12 +2564,14 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		use_keepalive = KEEPALIVE_ALWAYS;
 		execute_commands(commands, unpack_status, &si,
 				 &push_options);
+		sigchain_push(SIGPIPE, SIG_IGN);
 		if (pack_lockfile)
 			unlink_or_warn(pack_lockfile);
 		if (report_status_v2)
 			report_v2(commands, unpack_status);
 		else if (report_status)
 			report(commands, unpack_status);
+		sigchain_pop(SIGPIPE);
 		run_receive_hook(commands, "post-receive", 1,
 				 &push_options);
 		run_update_post_hook(commands);
-- 
2.30.2


  parent reply	other threads:[~2021-11-06 22:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-04 13:35 [RFC PATCH] receive-pack: run post-receive before reporting status Robin Jarry
2021-11-06  5:03 ` Ævar Arnfjörð Bjarmason
2021-11-06 21:32   ` Robin Jarry
2021-11-06 22:03 ` Robin Jarry [this message]
2021-11-09 21:10   ` [PATCH v2] receive-pack: ignore SIGPIPE while reporting status to client Junio C Hamano
2021-11-09 21:38     ` Robin Jarry
2021-11-09 23:03       ` Junio C Hamano
2021-11-10 10:35     ` [RFC PATCH] receive-pack: interrupt pre-receive when client disconnects Robin Jarry
2021-12-29 14:21       ` Robin Jarry
2021-11-10  9:29   ` [PATCH v3] receive-pack: ignore SIGPIPE while reporting status to client Robin Jarry
2021-11-18  9:36     ` Robin Jarry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211106220358.144886-1-robin@jarry.cc \
    --to=robin@jarry.cc \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jan.smets@nokia.com \
    --cc=nicolas.dichtel@6wind.com \
    --cc=peff@peff.net \
    --cc=stephen.morton@nokia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.