Message ID | dedaa3fa370ea9c4aeb1771b5568a7bef4065b04.1675113321.git.steffen@sdaoden.eu (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Herbert Xu |
Headers | show |
Series | Allow enabling job control without a tty in non-interactive mode.. | expand |
On Mon, Jan 30, 2023 at 10:15:40PM +0100, Steffen Nurpmeso wrote: > This is a take-over of the FreeBSD bin/sh > > commit cd60e2c67d52e1f957841af19128c7227880743a > Author: Jilles Tjoelker <jilles@FreeBSD.org> > AuthorDate: 2014-09-04 21:48:33 +0000 > Commit: Jilles Tjoelker <jilles@FreeBSD.org> > CommitDate: 2014-09-04 21:48:33 +0000 > > sh: Allow enabling job control without a tty in non-interactive mode. > > If no tty is available, 'set -m' is still useful to put jobs in their own > process groups. I don't see why dash needs this. Please keep in mind that one of the primary goals of dash it to be minimal. Thanks,
Herbert Xu wrote in <ZhD4JcqNct+rmxca@gondor.apana.org.au>: |On Mon, Jan 30, 2023 at 10:15:40PM +0100, Steffen Nurpmeso wrote: |> This is a take-over of the FreeBSD bin/sh |> |> commit cd60e2c67d52e1f957841af19128c7227880743a |> Author: Jilles Tjoelker <jilles@FreeBSD.org> |> AuthorDate: 2014-09-04 21:48:33 +0000 |> Commit: Jilles Tjoelker <jilles@FreeBSD.org> |> CommitDate: 2014-09-04 21:48:33 +0000 |> |> sh: Allow enabling job control without a tty in non-interactive \ |> mode. |> |> If no tty is available, 'set -m' is still useful to put jobs \ |> in their own |> process groups. | |I don't see why dash needs this. Please keep in mind that one |of the primary goals of dash it to be minimal. Isn't that standardized answer a bit misplaced for that patch? It was about Ganael Laplanche having problems with creating process groups in backgrounded jobs (2023-01-12): The problem is when the main script is started *in the background*. Can your test script successfully be run in the background with dash if $JOBMON is not empty ? I think you'll face the same problem as I do. I could not find Jilles' commit (cd60e2c) equivalent in dash code, so I presume enabling job control without a tty in non-interactive mode is just not possible with dash, am I right And i stated that another indirection fixes that problem and he then said It is not clear for me why that extra fork fixes the problem, but it works, thanks! Unfortunately, my initial goal is to get a new process group for later children processes, but FreeBSD's sh (as well as dash) requires 'set -m' to be executed from the first process [1]. The extra fork breaks that requirement. The following example shows that the new process' PGID remains the same as the initial shell: which let me baffled Thanks for this information, that i did not expect. Indeed .. it seems no new process group is used for the child shell. That thoroughly i have not looked. and So i think i am out of ideas except doing what Jilles suggested [Tjoelker] in the message, enwrapping the inner thing with sh -c '..'. And that seems to work a bit as and Ganael ended (without the patch) like Yes, I am afraid it's the only syntax that works (but it is not very convenient). etc. etc. etc. Full stop. Please consider this script: cat > t.sh <<'_EOT' set -m ( sleep 1 ) & i=$! set +m echo >&2 "inner Main shell has: $(ps -o pid,pgid $$)" echo >&2 "inner Sub-shell has: $(ps -o pid,pgid $i)" wait $i _EOT Current dash: #?0|kent:dash.git$ dash t.sh </dev/null >.X 2>&1 & [3] 28719 #?0|kent:dash.git$ ^RETURN [3]+ Stopped dash t.sh < /dev/null > .X 2>&1 #?0|kent:dash.git$ fg dash t.sh < /dev/null > .X 2>&1 Patched dash: #?0|kent:dash.git$ src/dash t.sh </dev/null >.X 2>&1 & [3] 28749 #?0|kent:dash.git$ ^RETURN [3] Done src/dash t.sh < /dev/null > .X 2>&1 Comparison: #?0|kent:dash.git$ bash t.sh </dev/null >.X 2>&1 & [3] 28766 #?0|kent:dash.git$ ^RETURN [3] Done bash t.sh < /dev/null > .X 2>&1 A nice Sunday everybody! Ciao from Germany, --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt)
On Sun, Apr 07, 2024 at 01:07:04AM +0200, Steffen Nurpmeso wrote: > Herbert Xu wrote in > <ZhD4JcqNct+rmxca@gondor.apana.org.au>: > |On Mon, Jan 30, 2023 at 10:15:40PM +0100, Steffen Nurpmeso wrote: > |> This is a take-over of the FreeBSD bin/sh > |> > |> commit cd60e2c67d52e1f957841af19128c7227880743a > |> Author: Jilles Tjoelker <jilles@FreeBSD.org> > |> AuthorDate: 2014-09-04 21:48:33 +0000 > |> Commit: Jilles Tjoelker <jilles@FreeBSD.org> > |> CommitDate: 2014-09-04 21:48:33 +0000 > |> > |> sh: Allow enabling job control without a tty in non-interactive \ > |> mode. > |> > |> If no tty is available, 'set -m' is still useful to put jobs \ > |> in their own > |> process groups. > | > |I don't see why dash needs this. Please keep in mind that one > |of the primary goals of dash it to be minimal. > > Isn't that standardized answer a bit misplaced for that patch? > It was about Ganael Laplanche having problems with creating > process groups in backgrounded jobs (2023-01-12): Sorry, I'd lost the context to the original discussion. I'll look into your patch again. Thanks,
diff --git a/src/jobs.c b/src/jobs.c index f3b9ffc285..db1f204045 100644 --- a/src/jobs.c +++ b/src/jobs.c @@ -125,7 +125,7 @@ STATIC int getstatus(struct job *); #if JOBS static int restartjob(struct job *, int); -static void xtcsetpgrp(int, pid_t); +static void xtcsetpgrp(pid_t); #endif STATIC void @@ -174,70 +174,84 @@ set_curjob(struct job *jp, unsigned mode) } } -#if JOBS /* * Turn job control on and off. - * - * Note: This code assumes that the third arg to ioctl is a character - * pointer, which is true on Berkeley systems but not System V. Since - * System V doesn't have job control yet, this isn't a problem now. - * - * Called with interrupts off. */ int jobctl; +#if JOBS +static void +jobctl_notty(void) +{ + if (ttyfd >= 0) { + close(ttyfd); + ttyfd = -1; + } + if (!iflag) { + setsignal(SIGTSTP); + setsignal(SIGTTOU); + setsignal(SIGTTIN); + jobctl = 1; + return; + } + sh_warnx("can't access tty; job control turned off"); + mflag = 0; +} + void setjobctl(int on) { - int fd; - int pgrp; + int i; if (on == jobctl || rootshell == 0) return; if (on) { - int ofd; - ofd = fd = sh_open(_PATH_TTY, O_RDWR, 1); - if (fd < 0) { - fd += 3; - while (!isatty(fd)) - if (--fd < 0) - goto out; + if (ttyfd != -1) + close(ttyfd); + if ((ttyfd = sh_open(_PATH_TTY, O_RDWR, 1)) < 0) { + i = 0; + while (i <= 2 && !isatty(i)) + i++; + if (i > 2 || + (ttyfd = fcntl(i, F_DUPFD_CLOEXEC, 10)) < 0) { + jobctl_notty(); + return; + } } - fd = savefd(fd, ofd); + ttyfd = savefd(ttyfd, ttyfd); do { /* while we are in the background */ - if ((pgrp = tcgetpgrp(fd)) < 0) { -out: - sh_warnx("can't access tty; job control turned off"); - mflag = on = 0; - goto close; + initialpgrp = tcgetpgrp(ttyfd); + if (initialpgrp < 0) { + jobctl_notty(); + return; } - if (pgrp == getpgrp()) - break; - killpg(0, SIGTTIN); - } while (1); - initialpgrp = pgrp; - + if (initialpgrp != getpgrp()) { + if (!iflag) { + initialpgrp = -1; + jobctl_notty(); + return; + } + kill(0, SIGTTIN); + continue; + } + } while (0); setsignal(SIGTSTP); setsignal(SIGTTOU); setsignal(SIGTTIN); - pgrp = rootpid; - setpgid(0, pgrp); - xtcsetpgrp(fd, pgrp); - } else { - /* turning job control off */ - fd = ttyfd; - pgrp = initialpgrp; - xtcsetpgrp(fd, pgrp); - setpgid(0, pgrp); + setpgid(0, rootpid); + tcsetpgrp(ttyfd, rootpid); + } else { /* turning job control off */ + setpgid(0, initialpgrp); + if (ttyfd >= 0) { + tcsetpgrp(ttyfd, initialpgrp); + close(ttyfd); + ttyfd = -1; + } setsignal(SIGTSTP); setsignal(SIGTTOU); setsignal(SIGTTIN); -close: - close(fd); - fd = -1; } - ttyfd = fd; jobctl = on; } #endif @@ -394,7 +408,7 @@ restartjob(struct job *jp, int mode) jp->state = JOBRUNNING; pgid = jp->ps->pid; if (mode == FORK_FG) - xtcsetpgrp(ttyfd, pgid); + xtcsetpgrp(pgid); killpg(pgid, SIGCONT); ps = jp->ps; i = jp->nprocs; @@ -457,6 +471,9 @@ showjob(struct output *out, struct job *jp, int mode) int indent; char s[80]; + if (!iflag) + return; + ps = jp->ps; if (mode & SHOW_PGID) { @@ -878,7 +895,7 @@ static void forkchild(struct job *jp, union node *n, int mode) /* This can fail because we are doing it in the parent also */ (void)setpgid(0, pgrp); if (mode == FORK_FG) - xtcsetpgrp(ttyfd, pgrp); + xtcsetpgrp(pgrp); setsignal(SIGTSTP); setsignal(SIGTTOU); } else @@ -1018,7 +1035,7 @@ waitforjob(struct job *jp) st = getstatus(jp); #if JOBS if (jp->jobctl) { - xtcsetpgrp(ttyfd, rootpid); + xtcsetpgrp(rootpid); /* * This is truly gross. * If we're doing job control, then we did a TIOCSPGRP which @@ -1508,12 +1525,15 @@ showpipe(struct job *jp, struct output *out) #if JOBS STATIC void -xtcsetpgrp(int fd, pid_t pgrp) +xtcsetpgrp(pid_t pgrp) { int err; + if (ttyfd < 0) + return; + sigblockall(NULL); - err = tcsetpgrp(fd, pgrp); + err = tcsetpgrp(ttyfd, pgrp); sigclearmask(); if (err)