Skip to content

Commit

Permalink
Implement support for POSIX_SPAWN_TCSETPGROUP (#438)
Browse files Browse the repository at this point in the history
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]: #79
[3]: https://www.mail-archive.com/[email protected]/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 66c3720.

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.
  • Loading branch information
JohnoKing authored Feb 3, 2022
1 parent 10e4ce5 commit 8e9ed5b
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 29 deletions.
4 changes: 2 additions & 2 deletions src/cmd/ksh93/include/defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
#define defs_h_defined

#include <ast.h>
#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).
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/path.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
38 changes: 37 additions & 1 deletion src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
# include <sys/resource.h>
#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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
33 changes: 18 additions & 15 deletions src/lib/libast/comp/spawnveg.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,14 @@

#include <ast.h>

#if _lib_spawnveg

NoN(spawnveg)

#else

#if _lib_posix_spawn > 1 /* reports underlying exec() errors */

#include <spawn.h>
#include <error.h>
#include <wait.h>

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;
Expand All @@ -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)
Expand All @@ -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))
{
Expand Down Expand Up @@ -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
Expand All @@ -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)
{
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -289,5 +294,3 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid)
#endif

#endif

#endif
2 changes: 1 addition & 1 deletion src/lib/libast/features/api
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
iff AST_API

ver ast 20220101
ver ast 20220201

api ast 20120528 regexec regnexec regrexec regsubexec strgrpmatch

Expand Down
5 changes: 3 additions & 2 deletions src/lib/libast/features/lib
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 <signal.h>
Expand Down Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion src/lib/libast/features/sys
Original file line number Diff line number Diff line change
Expand Up @@ -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*)
Expand Down
24 changes: 21 additions & 3 deletions src/lib/libast/man/spawnveg.3
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ spawnveg \- process spawn with process group and session control
.SH SYNOPSIS
.L "#include <ast.h>"
.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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/lib/libast/misc/procopen.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 8e9ed5b

Please sign in to comment.