[PATCH] Restore SIGCHLD to SIG_DFL where we care about waitpid().

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



It was reported that under one implementation of socks client,
"git clone" fails with "error: waitpid failed (No child
processes)", because "git" is spawned after setting SIGCHLD to
SIG_IGN.

Arguably it may be a broken setting, but we should protect
ourselves so that we can get reliable results from waitpid() for
the children we care about.

This patch resets SIGCHLD to SIG_DFL in three places:

 - connect.c::git_connect() - initiators of git native
   protocol transfer are covered with this.

 - daemon.c::main() - obviously.

 - merge-index.c::main() - obviously.

There are other programs that do fork() but do not waitpid():
http-push, imap-send.  upload-pack does not either, but in the
case of that program, each of the forked halves runs exec()
another program, so this change would not have much effect
there.

Signed-off-by: Junio C Hamano <junkio@xxxxxxx>
---

  Linus Torvalds <torvalds@xxxxxxxx> writes:

  > On Mon, 19 Jun 2006, Junio C Hamano wrote:
  >
  >> I do not offhand think of a place where we do fork() but not
  >> waitpid(), and it is very tempting to cheat and do that in the
  >> main(), since I do not see a downside to it.
  >
  > Yeah, it probably does make sense. That said, there are several "main()" 
  > functions, so you'd still end up having to verify that we catch all the 
  > paths.. Are all users of fork() built-in by now? 
 
  Not really.  But git native protocol initiators all use
  git_connect() so they are easy, and there are only few
  remaining ones that matter.

 connect.c     |    5 +++++
 daemon.c      |    5 +++++
 merge-index.c |    5 +++++
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/connect.c b/connect.c
index 52d709e..db7342e 100644
--- a/connect.c
+++ b/connect.c
@@ -581,6 +581,11 @@ int git_connect(int fd[2], char *url, co
 	enum protocol protocol = PROTO_LOCAL;
 	int free_path = 0;
 
+	/* Without this we cannot rely on waitpid() to tell
+	 * what happened to our children.
+	 */
+	signal(SIGCHLD, SIG_DFL);
+
 	host = strstr(url, "://");
 	if(host) {
 		*host = '\0';
diff --git a/daemon.c b/daemon.c
index 2f03f99..1067004 100644
--- a/daemon.c
+++ b/daemon.c
@@ -671,6 +671,11 @@ int main(int argc, char **argv)
 	int inetd_mode = 0;
 	int i;
 
+	/* Without this we cannot rely on waitpid() to tell
+	 * what happened to our children.
+	 */
+	signal(SIGCHLD, SIG_DFL);
+
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
 
diff --git a/merge-index.c b/merge-index.c
index 024196e..190e12f 100644
--- a/merge-index.c
+++ b/merge-index.c
@@ -99,6 +99,11 @@ int main(int argc, char **argv)
 {
 	int i, force_file = 0;
 
+	/* Without this we cannot rely on waitpid() to tell
+	 * what happened to our children.
+	 */
+	signal(SIGCHLD, SIG_DFL);
+
 	if (argc < 3)
 		usage("git-merge-index [-o] [-q] <merge-program> (-a | <filename>*)");
 
-- 
1.4.0.g275f

-
: 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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]