From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27879 invoked by alias); 19 Apr 2018 09:41:04 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 42684 Received: (qmail 6360 invoked by uid 1010); 19 Apr 2018 09:41:03 -0000 X-Qmail-Scanner-Diagnostics: from mailout2.w1.samsung.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(210.118.77.12):SA:0(-1.9/5.0):. Processed in 15.307362 secs); 19 Apr 2018 09:41:03 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS, SPF_PASS,T_DKIMWL_WL_HIGH,T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.1 X-Envelope-From: p.stephenson@samsung.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20180419094043euoutp02e970fcdc6c8d639e5ac29867973446e9~mzO1KflSz2588725887euoutp02Y DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1524130843; bh=DfEdBGrFq2ZR6VP+MlLnoavt+TSVdeN9tCYEfXt+ejU=; h=Date:From:To:Subject:In-reply-to:References:From; b=k/wdPLRf2g9J7u8S9NvexCUQALlPlPTFlnGC4VJKmjAPNc59sB1WDJvyKpDOm8pzZ 0v/qwUOkPJ8V4TImgcbhLWle/6HOK3ZKsqNjHseJPT99vg5FvvuCcAvw6IEr1DYgcI u086M0S70JnFFsJX7OQHrHPxdlx3Cy3YD2bVrwFA= X-AuditID: cbfec7f5-b45ff700000028a9-52-5ad8641a0e91 Date: Thu, 19 Apr 2018 10:40:39 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: "echo | ps -j $(:) | cat | cat | cat" runs components in different process groups Message-id: <20180419104039.7b86ed2b@camnpupstephen.cam.scsc.local> In-reply-to: <180417105243.ZM2929@torch.brasslantern.com> Organization: SCSC X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset="US-ASCII" Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrCIsWRmVeSWpSXmKPExsWy7djPc7pSKTeiDBrviFscbH7I5MDoserg B6YAxigum5TUnMyy1CJ9uwSujNP7d7IXnHGrmLHAo4HxlHEXIyeHhICJxJnJV9i6GLk4hARW MEpMet/JCOH0MkncXjyBFaaq9+AjRhBbSGAZo8Tsz1UQRdOYJD6fOs0E4ZxhlHh98AKUc4FR YvPPD0wgLSwCqhIzjuwBa2cTMJSYumk2mC0iIC5xdu15FhBbWCBB4uyeHWD1vALOEi0PO8Bs TgFLiS23nwPZHBz8AkISF5ptIS6ylzi65yRUuaDEj8n3wMYwC+hIbNv2mB3ClpfYvOYtM8g9 EgIT2CTa3oM4IM0uEicffoF6TVji1fEt7BC2jMTlyd0sEA3NjBJr799ng0j0MErMWhwKYVtL 9N2+yAixgU9i0rbpzCDHSQjwSnS0CUGUeEhcu/QZaqajxKq9bayQQHnGJvF/7hm2CYzys5Ac PgvJ4bOQHL6AkXkVo3hqaXFuemqxcV5quV5xYm5xaV66XnJ+7iZGYNyf/nf86w7GfX+SDjEK cDAq8fAm+F+PEmJNLCuuzD3EKMHBrCTCK34WKMSbklhZlVqUH19UmpNafIhRmoNFSZw3TqMu SkggPbEkNTs1tSC1CCbLxMEp1cBoGTDpZLuD3hkFh5r7as6nkhYc2nx45cX55sYV4mV+X6+p +IRp81reuX+q7Xwb75d13Tx76s+mZ19JTb2z7WXtow1eBgtOl/A1zHu/eS3T98eFMm0LdjJG suTes9ZaMIV/l9NCmZvrpXq239R/7K8lKK9y9N5Kcb9Lc2JUHqlpK6dWfdOctOa3EktxRqKh FnNRcSIArZB7sPcCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrILMWRmVeSWpSXmKPExsVy+t/xa7qSKTeiDE4sM7M42PyQyYHRY9XB D0wBjFFcNimpOZllqUX6dglcGaf372QvOONWMWOBRwPjKeMuRk4OCQETid6DjxhBbCGBJYwS N7/wdTFyAdkzmCQ+Nv9hh3DOMUpcmLSCCcK5wCix/vNtJpAWFgFViRlH9oC1swkYSkzdNBvM FhEQlzi79jwLiC0skCCx6MkEsDivgLNEy8MOsF5OAUuJLbefM0GsfsYmsXJTbRcjBwe/gJDE hWZbiOvsJY7uOckE0Soo8WPyPbCRzAJaEpu3NbFC2PISm9e8ZYYYoy5x4+5u9gmMQrOQtMxC 0jILScsCRuZVjCKppcW56bnFRnrFibnFpXnpesn5uZsYgQG77djPLTsYu94FH2IU4GBU4uFN 8L8eJcSaWFZcmXuIUYKDWUmEV/wsUIg3JbGyKrUoP76oNCe1+BCjNAeLkjjveYPKKCGB9MSS 1OzU1ILUIpgsEwenVANjpu1ntffiq0x/nsn6L1F+2bq07eny7Y5HgiSD+b5zxXDN/bH9xHWV 2H0eJe+Ou5n84ax57XfYZm/TPUVD0XDWGcubeTrMj8jYh7j8cOtefEuBefLu776bFOdE+nk+ PhawR23Z64jkA+G1BhNLNGJvfTwSwFtSpmPC8q0kzkby/ty4fo3Xp5YpsRRnJBpqMRcVJwIA 5jmF9lQCAAA= X-CMS-MailID: 20180419094041eucas1p1144690ed6cfe6adbfbb02287aaa8e1b5 X-Msg-Generator: CA CMS-TYPE: 201P X-CMS-RootMailID: 20180324221021epcas1p184507a6328dbd505b97db69c1f9d8194 X-RootMTR: 20180324221021epcas1p184507a6328dbd505b97db69c1f9d8194 References: <180323221959.ZM27569@torch.brasslantern.com> <20180324080514.txxyrb3qiztu4pqt@gmail.com> <180324150945.ZM32251@torch.brasslantern.com> <20180410124545.13fccd5d@camnpupstephen.cam.scsc.local> <20180410145926.64c4f671@camnpupstephen.cam.scsc.local> <180411151025.ZM19332@torch.brasslantern.com> <20180412172342.52df6b10@camnpupstephen.cam.scsc.local> <20180415162326.GA12549@chaz.gmail.com> <20180415185804.GB12549@chaz.gmail.com> <180416223910.ZM32002@torch.brasslantern.com> <20180417101947.5fd347df@camnpupstephen.cam.scsc.local> <180417090926.ZM2456@torch.brasslantern.com> <20180417173558.769503bd@camnpupstephen.cam.scsc.local> <180417105243.ZM2929@torch.brasslantern.com> >} The ampersand is therefore parsed rather late to be able to see you >need } this structure. So it probably needs some other trick --- a >different } list marker that causes a special null command akin to >time to do the } fork, for example, as it's easy to update word code >tokens when the } structure doesn't change. > > So what we'd need to do (?) is insert a no-op token in the wordcode at > the "front" of the parse, and update it to be an "in background" token > in the event the parse ends at "&" (or "&!" etc.) ... > >} The logic would then be within the exec code > > ... which would ignore the no-op or invoke an additional execcmd_bg() > to exec the pipeline after the fork. This strikes me as baroque enough we might just as well look at fixing it properly. This is likely to have obscure side effects of one sort or another, too, but with them being localised to execcmd_exec() I hope they'd be a bit more obvious than changing the execution structure... This is a first go that doesn't appear to be completely broken. Lots of rearranging and reindenting, very little novel code. I'm not sure how much git users feel like being guinea pigs. pws diff --git a/Src/exec.c b/Src/exec.c index f6c768f..5ea3e0b 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2707,6 +2707,77 @@ static void execcmd_getargs(LinkList preargs, LinkList args, int expand) } } +/**/ +static int execcmd_fork(Estate state, int how, int type, Wordcode varspc, + LinkList *filelistp, char *text, int oautocont) +{ + pid_t pid; + int synch[2], flags; + char dummy; + struct timeval bgtime; + + child_block(); + + if (pipe(synch) < 0) { + zerr("pipe failed: %e", errno); + return -1; + } else if ((pid = zfork(&bgtime)) == -1) { + close(synch[0]); + close(synch[1]); + lastval = 1; + errflag |= ERRFLAG_ERROR; + return -1; + } + if (pid) { + close(synch[1]); + read_loop(synch[0], &dummy, 1); + close(synch[0]); + if (how & Z_ASYNC) { + lastpid = (zlong) pid; + } else if (!jobtab[thisjob].stty_in_env && varspc) { + /* search for STTY=... */ + Wordcode p = varspc; + wordcode ac; + + while (wc_code(ac = *p) == WC_ASSIGN) { + if (!strcmp(ecrawstr(state->prog, p + 1, NULL), "STTY")) { + jobtab[thisjob].stty_in_env = 1; + break; + } + p += (WC_ASSIGN_TYPE(ac) == WC_ASSIGN_SCALAR ? + 3 : WC_ASSIGN_NUM(ac) + 2); + } + } + addproc(pid, text, 0, &bgtime); + if (oautocont >= 0) + opts[AUTOCONTINUE] = oautocont; + pipecleanfilelist(jobtab[thisjob].filelist, 1); + return pid; + } + + /* pid == 0 */ + close(synch[0]); + flags = ((how & Z_ASYNC) ? ESUB_ASYNC : 0) | ESUB_PGRP; + if ((type != WC_SUBSH) && !(how & Z_ASYNC)) + flags |= ESUB_KEEPTRAP; + if (type == WC_SUBSH && !(how & Z_ASYNC)) + flags |= ESUB_JOB_CONTROL; + *filelistp = jobtab[thisjob].filelist; + entersubsh(flags); + close(synch[1]); + + if (sigtrapped[SIGINT] & ZSIG_IGNORED) + holdintr(); +#ifdef HAVE_NICE + /* Check if we should run background jobs at a lower priority. */ + if ((how & Z_ASYNC) && isset(BGNICE)) + if (nice(5) < 0) + zwarn("nice(5) failed: %e", errno); +#endif /* HAVE_NICE */ + + return 0; +} + /* * Execute a command at the lowest level of the hierarchy. */ @@ -3074,6 +3145,21 @@ execcmd_exec(Estate state, Execcmd_params eparams, esprefork = (magic_assign || (isset(MAGICEQUALSUBST) && type != WC_TYPESET)) ? PREFORK_TYPESET : 0; + if (how & Z_ASYNC) { + text = getjobtext(state->prog, eparams->beg); + switch (execcmd_fork(state, how, type, varspc, &filelist, + text, oautocont)) { + case -1: + goto fatal; + case 0: + break; + default: + return; + } + forked = 1; + } else + text = NULL; + if (args) { if (eparams->htok) prefork(args, esprefork, NULL); @@ -3226,11 +3312,9 @@ execcmd_exec(Estate state, Execcmd_params eparams, } /* Get the text associated with this command. */ - if ((how & Z_ASYNC) || + if (!text && (!sfcontext && (jobbing || (how & Z_TIMED)))) text = getjobtext(state->prog, eparams->beg); - else - text = NULL; /* * Set up special parameter $_ @@ -3397,99 +3481,45 @@ execcmd_exec(Estate state, Execcmd_params eparams, * current shell. * **************************************************************************/ - if ((how & Z_ASYNC) || - (!do_exec && - (((is_builtin || is_shfunc) && output) || - (!is_cursh && (last1 != 1 || nsigtrapped || havefiles() || - fdtable_flocks))))) { - - pid_t pid; - int synch[2], flags; - char dummy; - struct timeval bgtime; - - child_block(); - - if (pipe(synch) < 0) { - zerr("pipe failed: %e", errno); - goto fatal; - } else if ((pid = zfork(&bgtime)) == -1) { - close(synch[0]); - close(synch[1]); - lastval = 1; - errflag |= ERRFLAG_ERROR; - goto fatal; - } - if (pid) { - - close(synch[1]); - read_loop(synch[0], &dummy, 1); - close(synch[0]); - if (how & Z_ASYNC) { - lastpid = (zlong) pid; - } else if (!jobtab[thisjob].stty_in_env && varspc) { - /* search for STTY=... */ - Wordcode p = varspc; - wordcode ac; - - while (wc_code(ac = *p) == WC_ASSIGN) { - if (!strcmp(ecrawstr(state->prog, p + 1, NULL), "STTY")) { - jobtab[thisjob].stty_in_env = 1; - break; - } - p += (WC_ASSIGN_TYPE(ac) == WC_ASSIGN_SCALAR ? - 3 : WC_ASSIGN_NUM(ac) + 2); - } + if (!forked) { + if (!do_exec && + (((is_builtin || is_shfunc) && output) || + (!is_cursh && (last1 != 1 || nsigtrapped || havefiles() || + fdtable_flocks)))) { + switch (execcmd_fork(state, how, type, varspc, &filelist, + text, oautocont)) { + case -1: + goto fatal; + case 0: + break; + default: + return; } - addproc(pid, text, 0, &bgtime); - if (oautocont >= 0) - opts[AUTOCONTINUE] = oautocont; - pipecleanfilelist(jobtab[thisjob].filelist, 1); - return; - } - /* pid == 0 */ - close(synch[0]); - flags = ((how & Z_ASYNC) ? ESUB_ASYNC : 0) | ESUB_PGRP; - if ((type != WC_SUBSH) && !(how & Z_ASYNC)) - flags |= ESUB_KEEPTRAP; - if (type == WC_SUBSH && !(how & Z_ASYNC)) - flags |= ESUB_JOB_CONTROL; - filelist = jobtab[thisjob].filelist; - entersubsh(flags); - close(synch[1]); - forked = 1; - if (sigtrapped[SIGINT] & ZSIG_IGNORED) - holdintr(); -#ifdef HAVE_NICE - /* Check if we should run background jobs at a lower priority. */ - if ((how & Z_ASYNC) && isset(BGNICE)) - if (nice(5) < 0) - zwarn("nice(5) failed: %e", errno); -#endif /* HAVE_NICE */ - - } else if (is_cursh) { - /* This is a current shell procedure that didn't need to fork. * - * This includes current shell procedures that are being exec'ed, * - * as well as null execs. */ - jobtab[thisjob].stat |= STAT_CURSH; - if (!jobtab[thisjob].procs) - jobtab[thisjob].stat |= STAT_NOPRINT; - if (is_builtin) - jobtab[thisjob].stat |= STAT_BUILTIN; - } else { - /* This is an exec (real or fake) for an external command. * - * Note that any form of exec means that the subshell is fake * - * (but we may be in a subshell already). */ - is_exec = 1; - /* - * If we are in a subshell environment anyway, say we're forked, - * even if we're actually not forked because we know the - * subshell is exiting. This ensures SHLVL reflects the current - * shell, and also optimises out any save/restore we'd need to - * do if we were returning to the main shell. - */ - if (type == WC_SUBSH) forked = 1; + } else if (is_cursh) { + /* This is a current shell procedure that didn't need to fork. * + * This includes current shell procedures that are being exec'ed, * + * as well as null execs. */ + jobtab[thisjob].stat |= STAT_CURSH; + if (!jobtab[thisjob].procs) + jobtab[thisjob].stat |= STAT_NOPRINT; + if (is_builtin) + jobtab[thisjob].stat |= STAT_BUILTIN; + } else { + /* This is an exec (real or fake) for an external command. * + * Note that any form of exec means that the subshell is fake * + * (but we may be in a subshell already). */ + is_exec = 1; + /* + * If we are in a subshell environment anyway, say we're forked, + * even if we're actually not forked because we know the + * subshell is exiting. This ensures SHLVL reflects the current + * shell, and also optimises out any save/restore we'd need to + * do if we were returning to the main shell. + */ + if (type == WC_SUBSH) + forked = 1; + } } if ((esglob = !(cflags & BINF_NOGLOB)) && args && eparams->htok) {