Skip to content

Commit

Permalink
Support glibc 2.35's posix_spawn_file_actions_addtcsetpgrp_np(3)
Browse files Browse the repository at this point in the history
This commit implements support for the glibc 2.35
posix_spawn_file_actions_addtcsetpgrp_np(3) extension[2][3],
updating spawnveg(3) to use the new function for setting the
terminal group. This was done with the intention of improving
performance in interactive shells without reintroducing previous
race conditions[4][5].

[1]: https://sourceware.org/pipermail/libc-alpha/2022-February/136040.html
[2]: https://sourceware.org/git/?p=glibc.git;a=commit;h=342cc934
[3]: https://sourceware.org/git/?p=glibc.git;a=commit;h=6289d28d
[4]: #79
[5]: https://www.mail-archive.com/[email protected]/msg00717.html

src/cmd/ksh93/sh/path.c:
- Tell spawnveg(3) to set the terminal process group when launching
  a child process in an interactive shell.

src/cmd/ksh93/sh/xec.c:
- If posix_spawn_file_actions_addtcsetpgrp_np(3) is available,
  allow use of spawnveg(3) (via sh_ntfork()) even with job control
  active.
- sh_ntfork(): Reimplement most of the 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_file_actions_addtcsetpgrp_np(3).
- Allow spawnveg to set the terminal process group when pgid == 0.
  This was necessary to avoid race conditions when using the new
  function.

src/lib/libast/features/lib:
- Detect posix_spawn_file_actions_addtcsetpgrp_np(3).
- 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/lib/libast/man/spawnveg.3:
- Document the changes to spawnveg(3) in the corresponding man
  page. Currently, it will only use the new tcfd argument if
  posix_spawn_file_actions_addtcsetpgrp_np(3) is supported. This
  could also be implemented for the fork(2) 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).
- Updated the man page to account for spawnveg's change in
  behavior.

Co-authored-by: Martijn Dekker <[email protected]>
  • Loading branch information
JohnoKing and McDutchie committed Feb 5, 2022
1 parent c7a774d commit 0863a8e
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 35 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_spawn_file_actions_addtcsetpgrp_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 @@ -3449,7 +3457,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_spawn_file_actions_addtcsetpgrp_np().
*/
static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int flag)
{
Expand All @@ -3461,6 +3470,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 @@ -3525,8 +3537,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 @@ -3587,6 +3612,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
57 changes: 36 additions & 21 deletions src/lib/libast/comp/spawnveg.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,33 +32,29 @@

#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;
posix_spawnattr_t attr;
int err, flags = 0;
pid_t pid;
posix_spawnattr_t attr;
#if _lib_posix_spawn_file_actions_addtcsetpgrp_np
posix_spawn_file_actions_t actions;
#endif

if (err = posix_spawnattr_init(&attr))
goto nope;
#if POSIX_SPAWN_SETSID
if (pgid == -1)
flags |= POSIX_SPAWN_SETSID;
else
#endif
if (pgid)
if (pgid && pgid != -1)
flags |= POSIX_SPAWN_SETPGROUP;
if (flags && (err = posix_spawnattr_setflags(&attr, flags)))
goto bad;
Expand All @@ -69,13 +65,33 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid)
if (err = posix_spawnattr_setpgroup(&attr, pgid))
goto bad;
}
#if _lib_posix_spawn_file_actions_addtcsetpgrp_np
if (tcfd >= 0)
{
if (err = posix_spawn_file_actions_init(&actions))
goto bad;
if (err = posix_spawn_file_actions_addtcsetpgrp_np(&actions, tcfd))
goto fail;
}
if (err = posix_spawn(&pid, path, (tcfd >= 0) ? &actions : NiL, &attr, argv, envv ? envv : environ))
#else
if (err = posix_spawn(&pid, path, NiL, &attr, argv, envv ? envv : environ))
#endif
{
if ((err != EPERM) || (err = posix_spawn(&pid, path, NiL, NiL, argv, envv ? envv : environ)))
goto bad;
goto fail;
}
#if _lib_posix_spawn_file_actions_addtcsetpgrp_np
if (tcfd >= 0)
posix_spawn_file_actions_destroy(&actions);
#endif
posix_spawnattr_destroy(&attr);
return pid;
fail:
#if _lib_posix_spawn_file_actions_addtcsetpgrp_np
if (tcfd >= 0)
posix_spawn_file_actions_destroy(&actions);
#endif
bad:
posix_spawnattr_destroy(&attr);
nope:
Expand All @@ -97,8 +113,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 +134,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 +172,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 +187,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 +233,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 +306,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_spawn_file_actions_addtcsetpgrp_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
20 changes: 17 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,24 @@ 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_spawn_file_actions_addtcsetpgrp_np
function.
When
.L tcfd
is
.LR >=0 ,
spawnveg will set the controlling terminal for the new process to
.IR tcfd .
.SH CAVEATS
If the
.I posix_spawn_file_actions_addtcsetpgrp_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

1 comment on commit 0863a8e

@McDutchie
Copy link

@McDutchie McDutchie commented on 0863a8e Feb 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @JohnoKing: this 1.0 branch commit is a combination of 8e9ed5b and f762a81 on the dev branch. I figured now that glibc 2.35 is released, this should see wider testing in the 93u+m 1.0.0-beta.3 release. Please let me know if you find any problems with this commit.

Please sign in to comment.