{"thread":{"id":"4574","subject":"[Q] what to do when waitpid() returns ECHILD under signal(SIGCHLD, SIG_IGN)?","startedAt":"2006-06-19T23:49:09Z","lastAt":"2006-06-20T16:09:21Z","messageCount":7,"participants":["Junio C Hamano","Linus Torvalds","Petr Baudis","Edgar Toernig"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"22084","messageId":"7vwtbc7ll6.fsf@assigned-by-dhcp.cox.net","threadId":"4574","inReplyTo":null,"subject":"[Q] what to do when waitpid() returns ECHILD under signal(SIGCHLD, SIG_IGN)?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-19T23:49:09Z","receivedAt":"2006-06-19T23:49:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Somebody I met last week in Japan reported that the socks client\nhe uses to cross the firewall to connect to git:// port from his\ncompany environment seems to do signal(SIGCHLD, SIG_IGN) before\nspawning git. When \"git clone\" is invoked this way, we get a\nmysterious failure.\n\nI can reproduce the problem without using funny socks client\nlike this:\n\n : gitster; trap '' SIGCHLD\n : gitster; git clone git://git.kernel.org/pub/scm/git/git.git/ foo.git\n error: waitpid failed (No child processes)\n fetch-pack from 'git://git.kernel.org/pub/scm/git/git.git/' failed.\n : gitster; ls foo.git\n ls: foo.git: No such file or directory\n\nWe could work this around by having signal(SIGCHLD, SIG_DFL)\nupfront in git.c::main(), but I am wondering what the standard\npractice for programs that use waitpid() call. Do they protect\nthemselves from this in order to reliably obtain child exit\nstatus? Or do they simply consider it is a user error to run a\nprogram that use waitpid() with SIGCHLD ignored?\n\nhttp://www.opengroup.org/onlinepubs/009695399/functions/waitpid.html\n\nexplicitly says this is an expected behaviour, so barfing upon\nECHILD sounds like a bug on our part.\n"},{"id":"22085","messageId":"Pine.LNX.4.64.0606191654590.5498@g5.osdl.org","threadId":"4574","inReplyTo":"7vwtbc7ll6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [Q] what to do when waitpid() returns ECHILD under signal(SIGCHLD, SIG_IGN)?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-06-19T23:57:28Z","receivedAt":"2006-06-19T23:57:28Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 19 Jun 2006, Junio C Hamano wrote:\n>\n> Somebody I met last week in Japan reported that the socks client\n> he uses to cross the firewall to connect to git:// port from his\n> company environment seems to do signal(SIGCHLD, SIG_IGN) before\n> spawning git.\n\nOk, that sounds pretty broken of it.\n\n> We could work this around by having signal(SIGCHLD, SIG_DFL)\n> upfront in git.c::main(), but I am wondering what the standard\n> practice for programs that use waitpid() call.\n\nWe need the status return, so failing on getting ECHILD is absolutely the \nright thing to do, because it implies that we don't know what the status \ncould have been.\n\nSo we need to reset SIGCHLD back to SIG_DFL (or catch it explicitly).\n\nWhether we want to do that in the main() routine or when we actually do \nthe fork() or whatever is a different issue.\n\n\t\tLinus\n"},{"id":"22088","messageId":"7vfyi07jf2.fsf@assigned-by-dhcp.cox.net","threadId":"4574","inReplyTo":"Pine.LNX.4.64.0606191654590.5498@g5.osdl.org","subject":"Re: [Q] what to do when waitpid() returns ECHILD under signal(SIGCHLD, SIG_IGN)?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-20T00:36:01Z","receivedAt":"2006-06-20T00:36:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Whether we want to do that in the main() routine or when we actually do \n> the fork() or whatever is a different issue.\n\nI do not offhand think of a place where we do fork() but not\nwaitpid(), and it is very tempting to cheat and do that in the\nmain(), since I do not see a downside to it.\n"},{"id":"22090","messageId":"Pine.LNX.4.64.0606191742470.5498@g5.osdl.org","threadId":"4574","inReplyTo":"7vfyi07jf2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [Q] what to do when waitpid() returns ECHILD under signal(SIGCHLD, SIG_IGN)?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-06-20T00:44:14Z","receivedAt":"2006-06-20T00:44:14Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 19 Jun 2006, Junio C Hamano wrote:\n>\n> Linus Torvalds writes:\n> \n> > Whether we want to do that in the main() routine or when we actually do \n> > the fork() or whatever is a different issue.\n> \n> I do not offhand think of a place where we do fork() but not\n> waitpid(), and it is very tempting to cheat and do that in the\n> main(), since I do not see a downside to it.\n\nYeah, it probably does make sense. That said, there are several \"main()\" \nfunctions, so you'd still end up having to verify that we catch all the \npaths.. Are all users of fork() built-in by now? \n\n\t\t\tLinus\n"},{"id":"22097","messageId":"7vslm04j2r.fsf@assigned-by-dhcp.cox.net","threadId":"4574","inReplyTo":"Pine.LNX.4.64.0606191742470.5498@g5.osdl.org","subject":"[PATCH] Restore SIGCHLD to SIG_DFL where we care about waitpid().","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-20T03:11:40Z","receivedAt":"2006-06-20T03:11:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It was reported that under one implementation of socks client,\n\"git clone\" fails with \"error: waitpid failed (No child\nprocesses)\", because \"git\" is spawned after setting SIGCHLD to\nSIG_IGN.\n\nArguably it may be a broken setting, but we should protect\nourselves so that we can get reliable results from waitpid() for\nthe children we care about.\n\nThis patch resets SIGCHLD to SIG_DFL in three places:\n\n - connect.c::git_connect() - initiators of git native\n protocol transfer are covered with this.\n\n - daemon.c::main() - obviously.\n\n - merge-index.c::main() - obviously.\n\nThere are other programs that do fork() but do not waitpid():\nhttp-push, imap-send. upload-pack does not either, but in the\ncase of that program, each of the forked halves runs exec()\nanother program, so this change would not have much effect\nthere.\n\nSigned-off-by: Junio C Hamano \n---\n\n Linus Torvalds writes:\n\n > On Mon, 19 Jun 2006, Junio C Hamano wrote:\n >\n >> I do not offhand think of a place where we do fork() but not\n >> waitpid(), and it is very tempting to cheat and do that in the\n >> main(), since I do not see a downside to it.\n >\n > Yeah, it probably does make sense. That said, there are several \"main()\" \n > functions, so you'd still end up having to verify that we catch all the \n > paths.. Are all users of fork() built-in by now? \n \n Not really. But git native protocol initiators all use\n git_connect() so they are easy, and there are only few\n remaining ones that matter.\n\n connect.c | 5 +++++\n daemon.c | 5 +++++\n merge-index.c | 5 +++++\n 3 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 52d709e..db7342e 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -581,6 +581,11 @@ int git_connect(int fd[2], char *url, co\n \tenum protocol protocol = PROTO_LOCAL;\n \tint free_path = 0;\n \n+\t/* Without this we cannot rely on waitpid() to tell\n+\t * what happened to our children.\n+\t */\n+\tsignal(SIGCHLD, SIG_DFL);\n+\n \thost = strstr(url, \"://\");\n \tif(host) {\n \t\t*host = '\\0';\ndiff --git a/daemon.c b/daemon.c\nindex 2f03f99..1067004 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -671,6 +671,11 @@ int main(int argc, char **argv)\n \tint inetd_mode = 0;\n \tint i;\n \n+\t/* Without this we cannot rely on waitpid() to tell\n+\t * what happened to our children.\n+\t */\n+\tsignal(SIGCHLD, SIG_DFL);\n+\n \tfor (i = 1; i < argc; i++) {\n \t\tchar *arg = argv[i];\n \ndiff --git a/merge-index.c b/merge-index.c\nindex 024196e..190e12f 100644\n--- a/merge-index.c\n+++ b/merge-index.c\n@@ -99,6 +99,11 @@ int main(int argc, char **argv)\n {\n \tint i, force_file = 0;\n \n+\t/* Without this we cannot rely on waitpid() to tell\n+\t * what happened to our children.\n+\t */\n+\tsignal(SIGCHLD, SIG_DFL);\n+\n \tif (argc < 3)\n \t\tusage(\"git-merge-index [-o] [-q] (-a | *)\");\n \n-- \n1.4.0.g275f\n"},{"id":"22137","messageId":"20060620125936.GQ2609@pasky.or.cz","threadId":"4574","inReplyTo":"7vslm04j2r.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Restore SIGCHLD to SIG_DFL where we care about waitpid().","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-06-20T12:59:38Z","receivedAt":"2006-06-20T12:59:38Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, Jun 20, 2006 at 05:11:40AM CEST, I got a letter\nwhere Junio C Hamano said that...\n> diff --git a/connect.c b/connect.c\n> index 52d709e..db7342e 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -581,6 +581,11 @@ int git_connect(int fd[2], char *url, co\n> \tenum protocol protocol = PROTO_LOCAL;\n> \tint free_path = 0;\n> \n> +\t/* Without this we cannot rely on waitpid() to tell\n> +\t * what happened to our children.\n> +\t */\n> +\tsignal(SIGCHLD, SIG_DFL);\n> +\n> \thost = strstr(url, \"://\");\n> \tif(host) {\n> \t\t*host = '\\0';\n\nIt would be nice if at this point of Git development we could already\nthink about libification when doing things like this (I'd like to do\nGit.pm as my little summer project this year). I'd make it part of the\nAPI that the calling application must not defer SIGCHLD, and conversely\nput the handler to all the main()s that reach git_connect().\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nA person is just about as big as the things that make them angry.\n"},{"id":"22148","messageId":"20060620180921.7bc1cb6c.froese@gmx.de","threadId":"4574","inReplyTo":"7vwtbc7ll6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [Q] what to do when waitpid() returns ECHILD under signal(SIGCHLD, SIG_IGN)?","fromName":"Edgar Toernig","fromEmail":"froese@gmx.de","sentAt":"2006-06-20T16:09:21Z","receivedAt":"2006-06-20T16:09:21Z","isPatch":false,"sender":{"key":"froese@gmx.de","avatar":null},"body":"Junio C Hamano wrote:\n>\n> Somebody I met last week in Japan reported that the socks client\n> he uses to cross the firewall to connect to git:// port from his\n> company environment seems to do signal(SIGCHLD, SIG_IGN) before\n> spawning git.\n\nSimilar problems occasionally happen with SIGPIPE. There are apps[1]\nthat spawn processes with SIGPIPE set to SIG_IGN giving unexpected\nresults, i.e. child processes that still try to produce output\n(getting EPIPE on every printf, but who checks printf errors?) even\nif the pipe-reader (e.g. their parent) is long gone.\n\nCiao, ET.\n\n[1] i.e. KDE had this bug - don't know if it's still there.\n"}]}