You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Overhaul spawnveg to use clone(2) and fix many spawning bugs (#888)
Alterations:
- Add a version of spawnveg that clones processes with clone(2)
outright. This method is generally faster than using posix_spawn,
and is also more portable (since Linux with musl libc *and*
NetBSD both provide this function). The code was initially based
on the _real_vfork spawnveg code previously removed in ef9a5c7
(unlike vfork(2), there is no problem with running code prior to
execve for clone with CLONE_VM|CLONE_VFORK).
- Remove the now dead code implementing tcsetpgrp via
posix_spawn_file_actions_addtcsetpgrp_np(). This code was only
ever used with glibc 2.35+. (Not only that, but the only other
platform that ever considered implementing such a call was
NetBSD[^1], where it appears to have been abandoned. This is
ironic considering ksh no longer uses posix_spawn on NetBSD as
of this commit.)
This code may be worth visiting if io_uring_spawn()[^2] is ever
actualized (that project appears to be quite inactive though).
- Bugfix: Don't set the terminal signals to their defaults in the
parent prior to calling spawnveg. This is the primary cause of a
lockup in the pty tests which can be reproduced as follows:
$ exec ksh
$ bin/shtests pty 2>/dev/null
^C ^C ^C (SIGINT doesn't work anymore and may segfault)
The correct way to go about dealing with SIGT* is to set those to
SIG_DFL in the child process (cf. _sh_fork()).
- Added support for tcsetpgrp to the fork fallback in spawnveg.
Some form of this appears to have already been attempted in AT&T
olden times, but that old version was broken and needed bugfixes
desperately.
- If the child needs tcsetpgrp, block the terminal signals in the
parent process via sigcritical(). The posix_spawn version
doesn't need this because posix_spawn will block signals
automatically and therefore doesn't need sigcritical.
- Now that the fork fallback for spawnveg works correctly in
interactive terminals, prefer that to the sh_fork() codepath on
operating systems without posix_spawn tcsetpgrp support. Even
though the underlying system call is still ultimately fork, the
sh_ntfork() codepath is faster than the traditional sh_fork
codepath. Benchmark:
$ time /tmp/ksh-devbranch -ic "for i in {1..10000}; do $(whence -p true); done"
real 0m03.302s
user 0m00.988s
sys 0m02.320s
$ time /tmp/ksh-newspawnveg -ic "for i in {1..10000}; do $(whence -p true); done"
real 0m03.160s
user 0m01.187s
sys 0m01.977s
- To that end, split up the spawnveg versions into spawnveg_fast
and spawnveg_slow. Choose the appropriate one when spawnveg is
called; this removes the need for the xec.c ifdef hackery.
- Removed the ntfork_tcpgrp ifdefs from xec.c; spawnveg can
handle it by itself now.
- Bugfix: With the spawnveg_fast and spawnveg_slow innovation,
spawnveg now always has support for setsid. It'll fallback to
fork if POSIX_SPAWN_SETSID and clone(2) aren't available.
- Bugfix: For the posix_spawn version of spawnveg, the flags should
be of the short type pursuant to the POSIX specification.
- Optimization: Use pipe2 in the fork fallback for spawnveg when
it's available to avoid two fcntl syscalls.
- Updated the spawnveg documentation to reflect the new changes.
- _spawnveg(): Restore the terminal process group immediately
after any failed spawn attempt, rather than only in sh_ntfork().
- Added a regression test for a crash that occurred because of
the erroneous tcsetpgrp placement in sh_ntfork().
- path_spawn(): Do not print an error message and longjmp away upon
encountering E2BIG or some other error; let the calling functions
take care of that.
- path_exec(): Added handling for E2BIG.
- Fixed a bug that caused sh_ntfork to leak file descriptors upon
encountering an error (re: 8f848bc). Reproducer:
#!/bin/ksh
ls /proc/$$/fd
redirect 2>/dev/null
touch /tmp/file
for i in {1..20}; do
command /tmp/file <(echo)
done
ls /proc/$$/fd
- Added regression tests for the leak that use 'command' and
'command -x'.
- Fixed an old regression test that was giving false negatives.
- Clarified the intent of the sigcrit nesting matches. The ksh2020
devs appeared to have been confused by this line, so some
additional clarification explaining the intent should be helpful
for posterity.
- path.sh: Added tests for the exit status of commands run when
forked with the & operator.
[^1]: https://blog.netbsd.org/tnf/entry/gsoc_reports_make_system_31
[^2]: https://lwn.net/Articles/908268/
# Same as above, but also test commands executed with command(1)
726
+
gotout=$(
727
+
ulimit -n 18 ||exit 0
728
+
set +x
729
+
PATH=/dev/null
730
+
LINENO=1
731
+
for((i=1; i<20; i++))
732
+
docommand notfound <(:)>(:)
733
+
done2>&1
734
+
exit
735
+
)
736
+
got=$?
737
+
((got==exp)) || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with command(1)" \
738
+
"(expected exit status $exp, got status $got)"
739
+
[[ $expout == "$gotout" ]] || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with command(1)" \
((got==exp)) || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with 'command -x'" \
755
+
"(expected exit status $exp, got status $got)"
756
+
[[ $expout == "$gotout" ]] || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with 'command -x'" \
0 commit comments