From 8e9ed5be147c16a3cd61e01070b8b2669db9172a Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Wed, 2 Feb 2022 16:34:52 -0800 Subject: [PATCH] Implement support for POSIX_SPAWN_TCSETPGROUP (#438) This commit implements support for the glibc POSIX_SPAWN_TCSETPGROUP and posix_spawnattr_tcsetpgrp_np extensions[1]. This was done with the intention of improving performance in interactive shells without reintroducing previous race conditions[2][3]. These changes were tested with glibc commit 342cc934. [1]: https://sourceware.org/git/?p=glibc.git;a=commit;h=342cc934a3bf74ac618e2318d738f22ac93257ba [2]: https://github.com/ksh93/ksh/issues/79 [3]: https://www.mail-archive.com/ast-developers@research.att.com/msg00717.html src/cmd/ksh93/sh/path.c: - Tell spawnveg to set the terminal process group when launching a child process in an interactive shell. src/cmd/ksh93/sh/xec.c: - Allow usage of spawnveg when posix_spawnattr_tcsetpgrp_np is available. - Reimplement most of the sh_ntfork() SIGTSTP handling code removed in commit 66c37202. src/lib/libast/comp/spawnveg.c, src/lib/libast/misc/procopen.c, src/lib/libast/features/sys: - Add support for POSIX_SPAWN_TCSETPGROUP as implemented in glibc. This code (with a few changes) might also work on QNX, but I've avoided changing anything on that OS since I don't have a QNX system to test with. src/lib/libast/man/spawnveg.3: - Document the changes to spawnveg(3) in the corresponding man page. Currently spawnveg will only use the new tcfd argument if posix_spawnattr_tcsetpgrp_np is supported. This could also be implemented for the fork fallback, but for now I've avoided changing that since actually using it in the fork code would likely require a lot of hackery to avoid attempting tcsetpgrp with vfork (the behavior of tcsetpgrp after vfork is not portable) and would only benefit systems that don't have posix_spawn and vfork (I can't recall any off the top of my head that would fall under that category). src/lib/libast/features/lib: - Detect the existence of the GNU posix_spawnattr_tcsetpgrp_np() extension. - Do not detect an OS spawnveg(3). With the API changes to spawnveg in this pull request ksh probably can't use the OS's spawnveg function anymore. (That's assuming anything else even provides a spawnveg function to begin with, which is unlikely.) src/lib/libast/features/api, src/cmd/ksh93/include/defs.h: - Bump libast version (20220101 => 20220201) due to the spawnveg(3) API change. --- src/cmd/ksh93/include/defs.h | 4 ++-- src/cmd/ksh93/sh/path.c | 2 +- src/cmd/ksh93/sh/xec.c | 38 +++++++++++++++++++++++++++++++++- src/lib/libast/comp/spawnveg.c | 33 +++++++++++++++-------------- src/lib/libast/features/api | 2 +- src/lib/libast/features/lib | 5 +++-- src/lib/libast/features/sys | 2 +- src/lib/libast/man/spawnveg.3 | 24 ++++++++++++++++++--- src/lib/libast/misc/procopen.c | 6 +++--- 9 files changed, 87 insertions(+), 29 deletions(-) diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index d66821adb9de..1a577ad1167a 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -29,8 +29,8 @@ #define defs_h_defined #include -#if !defined(AST_VERSION) || AST_VERSION < 20220101 -#error libast version 20220101 or later is required +#if !defined(AST_VERSION) || AST_VERSION < 20220201 +#error libast version 20220201 or later is required #endif #if !_lib_fork #error In 2021, ksh joined the 21st century and started requiring fork(2). diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index ab0a802c6906..945f34a848cb 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -147,7 +147,7 @@ static pid_t _spawnveg(const char *path, char* const argv[], char* const envp[], while(1) { sh_stats(STAT_SPAWN); - pid = spawnveg(path,argv,envp,pgid); + pid = spawnveg(path,argv,envp,pgid,job.jobcontrol?job.fd:-1); if(pid>=0 || errno!=EAGAIN) break; } diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index d020a8b2ed85..33599ed907aa 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -55,6 +55,10 @@ # include #endif +#if _lib_posix_spawn > 1 && _lib_posix_spawnattr_tcsetpgrp_np +#define _use_ntfork_tcpgrp 1 +#endif + #define SH_NTFORK SH_TIMING #define NV_BLTPFSH NV_ARRAY @@ -1613,7 +1617,11 @@ int sh_exec(register const Shnode_t *t, int flags) fifo_save_ppid = sh.current_pid; #endif #if SHOPT_SPAWN +#if _use_ntfork_tcpgrp + if(com) +#else if(com && !job.jobcontrol) +#endif /* _use_ntfork_tcpgrp */ { parent = sh_ntfork(t,com,&jobid,ntflag); if(parent<0) @@ -3444,7 +3452,8 @@ static void sigreset(int mode) /* * A combined fork/exec for systems with slow fork(). - * Incompatible with job control on interactive shells (job.jobcontrol). + * Incompatible with job control on interactive shells (job.jobcontrol) if + * the system does not support posix_spawnattr_tcsetpgrp_np(). */ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int flag) { @@ -3456,6 +3465,9 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int flag) char **arge, *path; volatile pid_t grp = 0; Pathcomp_t *pp; +#if _use_ntfork_tcpgrp + volatile int jobwasset=0; +#endif /* _use_ntfork_tcpgrp */ if(flag) { otype = savetype; @@ -3520,8 +3532,21 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int flag) } arge = sh_envgen(); sh.exitval = 0; +#if _use_ntfork_tcpgrp + if(job.jobcontrol) + { + signal(SIGTTIN,SIG_DFL); + signal(SIGTTOU,SIG_DFL); + signal(SIGTSTP,SIG_DFL); + jobwasset++; + } +#endif /* _use_ntfork_tcpgrp */ #ifdef JOBS +#if _use_ntfork_tcpgrp + if(sh_isstate(SH_MONITOR) && (job.jobcontrol || (otype&FAMP))) +#else if(sh_isstate(SH_MONITOR) && (otype&FAMP)) +#endif /* _use_ntfork_tcpgrp */ { if((otype&FAMP) || job.curpgid==0) grp = 1; @@ -3582,6 +3607,17 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int flag) sh_popcontext(buffp); if(buffp->olist) free_list(buffp->olist); +#if _use_ntfork_tcpgrp + if(jobwasset) + { + signal(SIGTTIN,SIG_IGN); + signal(SIGTTOU,SIG_IGN); + if(sh_isstate(SH_INTERACTIVE)) + signal(SIGTSTP,SIG_IGN); + else + signal(SIGTSTP,SIG_DFL); + } +#endif /* _use_ntfork_tcpgrp */ if(sigwasset) sigreset(1); /* restore ignored signals */ if(scope) diff --git a/src/lib/libast/comp/spawnveg.c b/src/lib/libast/comp/spawnveg.c index 20f880925637..882ac533b37d 100644 --- a/src/lib/libast/comp/spawnveg.c +++ b/src/lib/libast/comp/spawnveg.c @@ -32,12 +32,6 @@ #include -#if _lib_spawnveg - -NoN(spawnveg) - -#else - #if _lib_posix_spawn > 1 /* reports underlying exec() errors */ #include @@ -45,7 +39,7 @@ NoN(spawnveg) #include pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) +spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { int err, flags = 0; pid_t pid; @@ -59,7 +53,13 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) else #endif if (pgid) + { flags |= POSIX_SPAWN_SETPGROUP; +#if _lib_posix_spawnattr_tcsetpgrp_np + if (tcfd >= 0) + flags |= POSIX_SPAWN_TCSETPGROUP; +#endif + } if (flags && (err = posix_spawnattr_setflags(&attr, flags))) goto bad; if (pgid && pgid != -1) @@ -68,6 +68,10 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) pgid = 0; if (err = posix_spawnattr_setpgroup(&attr, pgid)) goto bad; +#if _lib_posix_spawnattr_tcsetpgrp_np + if (tcfd >= 0 && (err = posix_spawnattr_tcsetpgrp_np(&attr, tcfd))) + goto bad; +#endif } if (err = posix_spawn(&pid, path, NiL, &attr, argv, envv ? envv : environ)) { @@ -97,8 +101,9 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) #endif pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) +spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { + NOT_USED(tcfd); #if defined(P_DETACH) return spawnve(pgid ? P_DETACH : P_NOWAIT, path, argv, envv ? envv : environ); #else @@ -117,10 +122,11 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) */ pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) +spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { struct inheritance inherit; + NOT_USED(tcfd); inherit.flags = 0; if (pgid) { @@ -154,7 +160,7 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) */ pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) +spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { #if _lib_fork || _lib_vfork int n; @@ -169,6 +175,7 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) #endif #endif + NOT_USED(tcfd); if (!envv) envv = environ; #if _lib_spawnve @@ -214,11 +221,9 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) #if _lib_tcgetpgrp if (m) tcsetpgrp(2, pgid); -#else -#ifdef TIOCSPGRP +#elif defined(TIOCSPGRP) if (m) ioctl(2, TIOCSPGRP, &pgid); -#endif #endif } execve(path, argv, envv); @@ -289,5 +294,3 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid) #endif #endif - -#endif diff --git a/src/lib/libast/features/api b/src/lib/libast/features/api index 5158228fd2e8..edaf10f05205 100644 --- a/src/lib/libast/features/api +++ b/src/lib/libast/features/api @@ -1,6 +1,6 @@ iff AST_API -ver ast 20220101 +ver ast 20220201 api ast 20120528 regexec regnexec regrexec regsubexec strgrpmatch diff --git a/src/lib/libast/features/lib b/src/lib/libast/features/lib index 7531b9383684..2044a8c84519 100644 --- a/src/lib/libast/features/lib +++ b/src/lib/libast/features/lib @@ -36,7 +36,7 @@ lib mount,onexit,opendir,pathconf lib readlink,remove,rename,rewinddir,rindex,rmdir,setlocale lib setpgid,setpgrp,setpgrp2,setreuid,setsid,setuid,sigaction lib sigprocmask,sigsetmask,sigunblock,sigvec,socketpair -lib spawn,spawnve,spawnveg +lib spawn,spawnve lib strchr,strcoll,strdup,strerror,strcasecmp,strncasecmp,strrchr,strstr lib strmode,strxfrm,strftime,swab,symlink,sysconf,sysinfo,syslog lib telldir,tmpnam,tzset,universe,unlink,utime,wctype @@ -343,6 +343,7 @@ tst lib_posix_spawn unistd.h stdlib.h spawn.h -Dfork=______fork note{ posix_spaw _exit(n); } }end +lib posix_spawnattr_tcsetpgrp_np tst lib_spawn_mode unistd.h stdlib.h note{ first spawn arg is mode and it works }end execute{ #include @@ -642,7 +643,7 @@ tst - output{ printf("\n"); #endif - #if _lib_spawnveg || _lib_posix_spawn || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance || _lib_vfork && _real_vfork + #if _lib_posix_spawn || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance || _lib_vfork && _real_vfork printf("#if !_AST_no_spawnveg\n"); printf("#define _use_spawnveg 1\n"); printf("#endif\n"); diff --git a/src/lib/libast/features/sys b/src/lib/libast/features/sys index 6bab48802be6..a07334037cdb 100644 --- a/src/lib/libast/features/sys +++ b/src/lib/libast/features/sys @@ -195,7 +195,7 @@ extern setpgid int (pid_t, pid_t) extern setsid pid_t (void) extern setuid int (uid_t) extern sleep unsigned (unsigned int) -extern spawnveg pid_t (const char*, char* const[], char* const[], pid_t) +extern spawnveg pid_t (const char*, char* const[], char* const[], pid_t, int) extern srand void (unsigned int) extern strcasecmp int (const char*, const char*) extern strcat char* (char*, const char*) diff --git a/src/lib/libast/man/spawnveg.3 b/src/lib/libast/man/spawnveg.3 index 7778d82ba1e9..0b20a97b9c17 100644 --- a/src/lib/libast/man/spawnveg.3 +++ b/src/lib/libast/man/spawnveg.3 @@ -42,7 +42,7 @@ spawnveg \- process spawn with process group and session control .SH SYNOPSIS .L "#include " .sp -.L "int spawnveg(const char* command, char** argv, char** envv, pid_t pgid);" +.L "int spawnveg(const char* command, char** argv, char** envv, pid_t pgid, int tcfd);" .SH DESCRIPTION .L spawnveg combines @@ -81,10 +81,28 @@ The new process becomes a process group leader. .L >1 The new process joins the process group .IR pgid . -.SH CAVEATS +.PP The +.L tcfd +argument is currently only used if the operating system supports the +.I posix_spawnattr_tcsetpgrp_np +function. +When +.L tcfd +is +.L >=0 +and +.L pgid +is +.LR >0 , +spawnveg will set the controlling terminal for the new process to +.IR tcfd . +.SH CAVEATS +If the +.I posix_spawnattr_tcsetpgrp_np +function is not available, then .L spawnveg -function cannot set the terminal process group. +cannot reliably set the terminal process group. As a result, it is incompatible with job control when used with terminals. Additionally, if the .L POSIX_SPAWN_SETSID diff --git a/src/lib/libast/misc/procopen.c b/src/lib/libast/misc/procopen.c index 2bced4de35de..175c9d5796fe 100644 --- a/src/lib/libast/misc/procopen.c +++ b/src/lib/libast/misc/procopen.c @@ -2,7 +2,7 @@ * * * This software is part of the ast package * * Copyright (c) 1985-2012 AT&T Intellectual Property * -* Copyright (c) 2020-2021 Contributors to ksh 93u+m * +* Copyright (c) 2020-2022 Contributors to ksh 93u+m * * and is licensed under the * * Eclipse Public License, Version 1.0 * * by AT&T Intellectual Property * @@ -744,7 +744,7 @@ sfsync(sfstderr); if (forked || (flags & PROC_OVERLAY)) execve(path, p, environ); #if _use_spawnveg - else if ((proc->pid = spawnveg(path, p, environ, proc->pgrp)) != -1) + else if ((proc->pid = spawnveg(path, p, environ, proc->pgrp, -1)) != -1) goto cleanup; #endif if (errno != ENOEXEC) @@ -773,7 +773,7 @@ sfsync(sfstderr); execve(env + 2, p, environ); #if _use_spawnveg else - proc->pid = spawnveg(env + 2, p, environ, proc->pgrp); + proc->pid = spawnveg(env + 2, p, environ, proc->pgrp, -1); #endif cleanup: if (forked)