From owner-svn-src-head@FreeBSD.ORG Tue Aug 13 21:00:48 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 182F76DA; Tue, 13 Aug 2013 21:00:48 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 9A051252A; Tue, 13 Aug 2013 21:00:47 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 332DE358C67; Tue, 13 Aug 2013 23:00:46 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id F1AC028494; Tue, 13 Aug 2013 23:00:45 +0200 (CEST) Date: Tue, 13 Aug 2013 23:00:45 +0200 From: Jilles Tjoelker To: Peter Wemm Subject: Re: svn commit: r254297 - head/lib/libc/stdlib Message-ID: <20130813210045.GB68244@stack.nl> References: <201308132038.r7DKcuH7082570@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201308132038.r7DKcuH7082570@svn.freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Aug 2013 21:00:48 -0000 On Tue, Aug 13, 2013 at 08:38:56PM +0000, Peter Wemm wrote: > Author: peter > Date: Tue Aug 13 20:38:55 2013 > New Revision: 254297 > URL: http://svnweb.freebsd.org/changeset/base/254297 > Log: > vfork(2) was listed as deprecated in 1994 (r1573) and was the false > reports of its impending demise were removed in 2009 (r199257). > However, in 1996 (r16117) system(3) was switched from vfork(2) to > fork(2) based partly on this. Switch back to vfork(2). This has a > dramatic effect in cases of extreme mmap use - such as excessive > abuse (500+) of shared libraries. > popen(3) has used vfork(2) for a while. vfork(2) isn't going anywhere. > Modified: > head/lib/libc/stdlib/system.c > Modified: head/lib/libc/stdlib/system.c > ============================================================================== > --- head/lib/libc/stdlib/system.c Tue Aug 13 20:33:50 2013 (r254296) > +++ head/lib/libc/stdlib/system.c Tue Aug 13 20:38:55 2013 (r254297) > @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > #include > #include > #include > @@ -56,37 +57,36 @@ __system(const char *command) > if (!command) /* just checking... */ > return(1); > > - /* > - * Ignore SIGINT and SIGQUIT, block SIGCHLD. Remember to save > - * existing signal dispositions. > - */ > - ign.sa_handler = SIG_IGN; > - (void)sigemptyset(&ign.sa_mask); > - ign.sa_flags = 0; > - (void)_sigaction(SIGINT, &ign, &intact); > - (void)_sigaction(SIGQUIT, &ign, &quitact); > (void)sigemptyset(&newsigblock); > (void)sigaddset(&newsigblock, SIGCHLD); > (void)_sigprocmask(SIG_BLOCK, &newsigblock, &oldsigblock); > - switch(pid = fork()) { > + switch(pid = vfork()) { > case -1: /* error */ > - break; > + (void)_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); > + return (-1); > case 0: /* child */ > /* > * Restore original signal dispositions and exec the command. > */ > - (void)_sigaction(SIGINT, &intact, NULL); > - (void)_sigaction(SIGQUIT, &quitact, NULL); > (void)_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); > execl(_PATH_BSHELL, "sh", "-c", command, (char *)NULL); > _exit(127); > - default: /* parent */ > - savedpid = pid; > - do { > - pid = _wait4(savedpid, &pstat, 0, (struct rusage *)0); > - } while (pid == -1 && errno == EINTR); > - break; > } > + /* > + * If we are running means that the child has either completed > + * its execve, or has failed. > + * Block SIGINT/QUIT because sh -c handles it and wait for > + * it to clean up. > + */ > + memset(&ign, 0, sizeof(ign)); > + ign.sa_handler = SIG_IGN; > + (void)sigemptyset(&ign.sa_mask); > + (void)_sigaction(SIGINT, &ign, &intact); > + (void)_sigaction(SIGQUIT, &ign, &quitact); > + savedpid = pid; > + do { > + pid = _wait4(savedpid, &pstat, 0, (struct rusage *)0); > + } while (pid == -1 && errno == EINTR); > (void)_sigaction(SIGINT, &intact, NULL); > (void)_sigaction(SIGQUIT, &quitact, NULL); > (void)_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); This is definitely good, but I think ignoring SIGINT/SIGQUIT only after the vfork() puts back an old bug. The ignore was deliberately moved to before fork(). You can avoid it more simply by adding SIGINT and SIGQUIT to the signal mask above. The kernel then keeps any SIGINT/SIGQUIT pending until the signal is ignored after vfork(). -- Jilles Tjoelker