git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Jeff King" <peff@peff.net>,
	git@vger.kernel.org, "Jonathan Nieder" <jrnieder@gmail.com>,
	"Erik Faye-Lund" <kusmabite@gmail.com>,
	"Ilari Liusvaara" <ilari.liusvaara@elisanet.fi>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 2/5 v2] dashed externals: kill children on exit
Date: Sun, 8 Jan 2012 21:41:09 +0100	[thread overview]
Message-ID: <20120108204109.GA3394@ecki.lan> (raw)
In-Reply-To: <7v4nw6hfpy.fsf@alter.siamese.dyndns.org>

Several git commands are so-called dashed externals, that is commands
executed as a child process of the git wrapper command. If the git
wrapper is killed by a signal, the child process will continue to run.
This is different from internal commands, which always die with the git
wrapper command.

Enable the recently introduced cleanup mechanism for child processes in
order to make dashed externals act more in line with internal commands.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

On Sat, Jan 07, 2012 at 10:26:49PM -0800, Junio C Hamano wrote:
> 
> Yeah, I agree 100% with that reasoning. I seem to recall that was how this
> commit was done in what I privately reviewed after Clemens announced his
> github branch?

What I had previously enabled child cleanup for all callers of
run_command_v_opt. There is quite a few of those as well, most of them
not related to dashed externals at all.

So I reworked that patch a bit to enable cleanup only for dashed
externals. This is a replacement for "[PATCH 2/5] run-command: kill
children on exit by default".

I also re-added Jeff's "send-pack: kill pack-objects helper on signal or
exit" and I dropped the extraneous newline that Erik spotted in
"[PATCH 1/5] run-command: optionally kill children on exit".

I have pushed the reworked series to cb/git-daemon-tests on my github
repo.

 git.c         |    2 +-
 run-command.c |    1 +
 run-command.h |    1 +
 3 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git.c b/git.c
index fb9029c..3805616 100644
--- a/git.c
+++ b/git.c
@@ -495,7 +495,7 @@ static void execv_dashed_external(const char **argv)
 	 * if we fail because the command is not found, it is
 	 * OK to return. Otherwise, we just pass along the status code.
 	 */
-	status = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE);
+	status = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT);
 	if (status >= 0 || errno != ENOENT)
 		exit(status);
 
diff --git a/run-command.c b/run-command.c
index fff9073..90bfd8c 100644
--- a/run-command.c
+++ b/run-command.c
@@ -496,6 +496,7 @@ static void prepare_run_command_v_opt(struct child_process *cmd,
 	cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
 	cmd->silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;
 	cmd->use_shell = opt & RUN_USING_SHELL ? 1 : 0;
+	cmd->clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
 }
 
 int run_command_v_opt(const char **argv, int opt)
diff --git a/run-command.h b/run-command.h
index 2a69466..44f7d2b 100644
--- a/run-command.h
+++ b/run-command.h
@@ -53,6 +53,7 @@ extern int run_hook(const char *index_file, const char *name, ...);
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
 #define RUN_SILENT_EXEC_FAILURE 8
 #define RUN_USING_SHELL 16
+#define RUN_CLEAN_ON_EXIT 32
 int run_command_v_opt(const char **argv, int opt);
 
 /*
-- 
1.7.8

  reply	other threads:[~2012-01-08 20:49 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-01  1:26 [PATCH] transport: do not allow to push over git:// protocol Nguyễn Thái Ngọc Duy
2011-10-01  2:25 ` Ilari Liusvaara
2011-10-01  4:27   ` Nguyen Thai Ngoc Duy
2011-10-01  5:29   ` Jonathan Nieder
2011-10-03  9:12     ` Nguyen Thai Ngoc Duy
     [not found] ` <20111002223805.0bd6678b@zappedws>
2011-10-02 21:11   ` Nguyen Thai Ngoc Duy
2011-10-03  7:42 ` Jeff King
2011-10-03  8:44   ` Johannes Sixt
2011-10-03  9:39     ` Jeff King
2011-10-03  9:44       ` Nguyen Thai Ngoc Duy
2011-10-03  9:47         ` Jeff King
2011-10-03  9:52           ` Nguyen Thai Ngoc Duy
2011-10-03 11:13         ` Jonathan Nieder
2011-10-03 19:28           ` [PATCH] daemon: print "access denied" if a service does not work Nguyễn Thái Ngọc Duy
2011-10-03 19:54             ` Jonathan Nieder
2011-10-03 19:57             ` Junio C Hamano
2011-10-03 21:55               ` [PATCH] daemon: return "access denied" if a service is not allowed Nguyễn Thái Ngọc Duy
2011-10-03 22:20                 ` Junio C Hamano
2011-10-12 20:09                 ` Jeff King
2011-10-13  2:14                   ` Jonathan Nieder
2011-10-13  4:45                   ` Nguyen Thai Ngoc Duy
2011-10-13  5:59                     ` Jonathan Nieder
2011-10-13  6:56                       ` Nguyen Thai Ngoc Duy
2011-10-13  7:02                         ` Nguyen Thai Ngoc Duy
2011-10-13 18:28                     ` Jeff King
2011-10-14  5:01                       ` Junio C Hamano
2011-10-14 13:10                         ` Jeff King
2011-10-14 19:23                           ` Jeff King
2011-10-14 19:27                             ` Jeff King
2011-10-14 20:24                               ` Junio C Hamano
2011-10-14 20:34                                 ` Jeff King
2011-10-14 20:48                                   ` Junio C Hamano
2011-10-14 21:05                                     ` Jeff King
2011-10-14 21:06                                       ` Jonathan Nieder
2011-10-14 21:20                               ` Jonathan Nieder
2011-10-14 21:02                             ` Jonathan Nieder
2011-10-14 21:12                               ` Jeff King
2011-10-14 21:19                                 ` [PATCHv3] daemon: give friendlier error messages to clients Jeff King
2011-10-14 21:52                                   ` Junio C Hamano
2011-10-14 23:39                                   ` Sitaram Chamarty
2011-10-15  5:55                                     ` Junio C Hamano
2011-10-15  7:09                                       ` Sitaram Chamarty
2011-10-15  8:16                                         ` Jakub Narebski
2011-10-15  8:26                                           ` Jonathan Nieder
2011-10-15 20:13                                             ` Junio C Hamano
2011-10-15 22:17                                               ` Jonathan Nieder
2011-10-16  1:51                                                 ` Sitaram Chamarty
2011-10-15  0:51                                   ` Nguyen Thai Ngoc Duy
2011-10-16 22:11                                   ` [PATCH 1/2] daemon: add tests Clemens Buchacher
2011-10-16 22:11                                     ` [PATCH 2/2] daemon: report permission denied error to clients Clemens Buchacher
2011-10-17  2:09                                       ` Jeff King
2011-10-17 19:48                                         ` Clemens Buchacher
2011-10-17 19:51                                           ` Jeff King
2011-10-17 21:03                                         ` Junio C Hamano
2011-10-18 20:41                                           ` Clemens Buchacher
2011-10-19  6:33                                             ` Clemens Buchacher
2011-10-17 19:58                                       ` [PATCH v2 " Clemens Buchacher
2011-10-21 19:25                                         ` Junio C Hamano
2011-10-17  2:01                                     ` [PATCH 1/2] daemon: add tests Jeff King
2011-10-17 19:55                                       ` [PATCH] use test number as port number Clemens Buchacher
2011-10-17 20:57                                         ` Junio C Hamano
2011-10-18 20:09                                           ` Clemens Buchacher
2011-10-17 20:05                                       ` [PATCH 1/2] daemon: add tests Clemens Buchacher
2011-10-17 20:08                                         ` Jeff King
2012-01-02  9:25                                     ` Jonathan Nieder
2012-01-02 19:47                                       ` Clemens Buchacher
2012-01-03 19:18                                         ` Jeff King
2012-01-03 19:34                                       ` Junio C Hamano
2012-01-04 15:55                                         ` Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 1/6] t5550: repack everything into one file Clemens Buchacher
2012-01-04 18:05                                             ` Junio C Hamano
2012-01-04 15:55                                           ` [PATCH 2/6] daemon: add tests Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 3/6] avoid use of pkill Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 4/6] explain expected exit code Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 5/6] t5570: repack everything into one file Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 6/6] chmod: use lower-case x Clemens Buchacher
2012-01-04 18:00                                           ` [PATCH 1/2] daemon: add tests Junio C Hamano
2012-01-04 20:13                                             ` Junio C Hamano
2012-01-04 20:40                                             ` Clemens Buchacher
2012-01-04 22:15                                               ` Junio C Hamano
2012-01-04 22:26                                                 ` Jeff King
2012-01-05  0:07                                                   ` Clemens Buchacher
2012-01-05  0:24                                                     ` Junio C Hamano
2012-01-05  0:38                                                       ` Clemens Buchacher
2012-01-05  2:55                                                     ` Jeff King
2012-01-05 16:06                                                       ` Clemens Buchacher
2012-01-06 15:52                                                         ` Jeff King
2012-01-06 19:48                                                           ` Clemens Buchacher
2012-01-06 22:32                                                             ` Jeff King
2012-01-07 11:54                                                               ` [PATCH] credentials: unable to connect to cache daemon Clemens Buchacher
2012-01-07 14:55                                                                 ` Jeff King
2012-01-06 22:49                                                             ` [PATCH 1/2] daemon: add tests Junio C Hamano
2012-01-07 11:42                                                               ` Clemens Buchacher
2012-01-07 11:42                                                                 ` [PATCH 1/5] run-command: optionally kill children on exit Clemens Buchacher
2012-01-07 12:45                                                                   ` Erik Faye-Lund
2012-01-08 20:56                                                                     ` Clemens Buchacher
2012-01-07 14:41                                                                   ` Jeff King
2012-01-07 11:42                                                                 ` [PATCH 2/5] run-command: kill children on exit by default Clemens Buchacher
2012-01-07 14:50                                                                   ` Jeff King
2012-01-08  6:26                                                                     ` Junio C Hamano
2012-01-08 20:41                                                                       ` Clemens Buchacher [this message]
2012-01-08 21:07                                                                         ` [PATCH 2/5 v2] dashed externals: kill children on exit Jeff King
2012-01-07 11:42                                                                 ` [PATCH 3/5] git-daemon: add tests Clemens Buchacher
2012-01-07 11:42                                                                 ` [PATCH 4/5] git-daemon: produce output when ready Clemens Buchacher
2012-01-07 11:42                                                                 ` [PATCH 5/5] git-daemon tests: wait until daemon is ready Clemens Buchacher
2012-01-05  2:24                                                   ` [PATCH 1/2] daemon: add tests Jakub Narebski
2012-01-05  2:51                                                     ` Jeff King
2012-01-06 23:35                                                       ` Jakub Narebski
2012-01-07 11:46                                                         ` Clemens Buchacher
2012-01-06  6:17                                           ` Brian Gernhardt
2011-10-03  9:49     ` [PATCH] transport: do not allow to push over git:// protocol Jakub Narebski
2011-10-03 10:02       ` Jeff King
2011-10-03 11:01   ` Ilari Liusvaara
2011-10-03 11:26     ` [PATCH] Support ERR in remote archive like in fetch/push Jonathan Nieder
2011-10-03 11:45       ` René Scharfe
2011-10-03 18:13     ` [PATCH] transport: do not allow to push over git:// protocol Nguyen Thai Ngoc Duy
2011-10-03 20:27       ` Junio C Hamano

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=20120108204109.GA3394@ecki.lan \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ilari.liusvaara@elisanet.fi \
    --cc=jrnieder@gmail.com \
    --cc=kusmabite@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).