Page MenuHomeFreeBSD

sh: add the -S flag, to call setsid() when forking subshells
Needs RevisionPublic

Authored by allanjude on Mar 11 2021, 12:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Sep 22, 10:32 PM
Unknown Object (File)
Thu, Sep 19, 2:17 PM
Unknown Object (File)
Sep 5 2024, 1:51 AM
Unknown Object (File)
Aug 17 2024, 7:23 AM
Unknown Object (File)
Jul 27 2024, 4:53 AM
Unknown Object (File)
May 22 2024, 12:24 AM
Unknown Object (File)
Apr 26 2024, 9:05 AM
Unknown Object (File)
Apr 25 2024, 8:02 PM
Subscribers

Details

Reviewers
jilles
dteske
Group Reviewers
manpages
Summary

Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 37757
Build 34646: arc lint + arc unit

Event Timeline

0mp added inline comments.
bin/sh/jobs.c
915

Shouldn't we check the return value for errors?

bin/sh/sh.1
308

Perhaps we could add some explanation why this option might be useful

jilles requested changes to this revision.Mar 11 2021, 9:23 PM

Perhaps it makes more sense to implement this as a separate program that performs setsid() and then execvp(). This is a bit like daemon, but not quite, since daemon also forks and makes it hard to track the child process. As noted in the man page, sh tends not to implement many extensions. Extensions like set -o trapsasync and set -o pipefail are different from this one because they are hard to implement outside the shell or in a shell script. Of these, set -o pipefail is particularly simple to implement in the shell.

bin/sh/jobs.c
915

It would feel safer to add this further down (but still before the setpgid() from job control). In particular, doing things that may throw exceptions before the handler = &main_handler; below may cause exception handlers to be run twice (in parent and child). This might be problematic because, for example, histedit.c has an exception handler that deletes a temporary file. Although system calls such as setsid() do not throw exceptions, a surprising amount of code in sh may throw exceptions.

915

Doing this here directly after fork() ensures that setsid() will not fail, but in some situations it will be poorly predictable whether a new session will be created, since sh treats this as an implementation detail.

For example, a final subshell or program may or may not be executed in a new process:

$ sh -c 'ps -o comm= -p $$'
ps
$ sh -c 'trap "" 0; ps -o comm= -p $$'
ps
$ sh -c 'trap : 0; ps -o comm= -p $$'
sh
$ sh -c ':; (:; ps -o comm= -p $$)'
ps
$ sh -c 'if :; then ps -o comm= -p $$; fi'
ps

certain simple commands may use vfork() instead of fork(), and command substitutions may be executed entirely in-process, using vfork() or using fork().

This is why POSIX talks about "shell environments" and not "child processes" when discussing the effect of () and the like.

One completely predictable fork() is the one caused by the running something in the background using the & control operator. With non-interactive job control this is a fixed pattern set -m; X & set +m. A new option might only be active in this predictable case.

915

Since there is no way to move a process to another existing session and all members of a pipeline are children of the main shell process, a command like set -o setsid; A | B | C & will unavoidably create three sessions. This differs from job control where the shell puts A, B and C into the same process group.

On the other hand, set -o setsid; (A | B | C) & adds an intermediate process which will allow A, B and C to be in a single session.

This comment does not suggest an immediate direction in this.

bin/sh/options.c
300–301

These lines do nothing and are not needed. The above four turn off -V when -E is enabled, and vice versa.

bin/sh/options.h
71

Only adding a long name (like nolog and pipefail) feels a bit safer against conflict with new options created elsewhere.

bin/sh/sh.1
306

This clause does not seem to make much sense, since creating a new session implies creating a new process group, so job control would not add anything to the new option. Also, processes from a different session are immune to SIGTTIN and the like.

This revision now requires changes to proceed.Mar 11 2021, 9:23 PM