Monitor mode handling (bug ?)

Monitor mode handling (bug ?)

[Ganael Laplanche]

Hello,

Digging into a problem with a project of mine [1], I found dash (0.5.12) 
behaves differently from other POSIX-compliant shells when dealing with 
monitor mode when running in the background.

In the following example, dash immediately sends a SIGTTIN signal that stops 
the process group:

$ cat test.sh
set -m
$ dash test.sh &
[1]  + suspended (tty input)  dash test.sh

while other shells do not:

$ bash test.sh &
[1]  + done       bash test.sh
$ sh test.sh & # (/bin/sh from FreeBSD 13.1-RELEASE)
[1]  + done       sh test.sh
$ ksh93 test.sh &
[1]  + done       ksh93 test.sh

Is that an intended behaviour ?

Also, the following test leads to dash looping indefinitely and eating 100% 
CPU:

$ cat test2.sh
trap '' 21
set -m
$ dash test2.sh &

It remains stuck within the following loop:

https://git.kernel.org/pub/scm/utils/dash/dash.git/tree/src/jobs.c?
h=v0.5.12#n208

Weird... Is that a dash bug or am I missing something?

Best regards,

Ganael.

[1] See https://github.com/martymac/fpart/issues/43 for more context

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Jilles Tjoelker]

On Mon, Jan 09, 2023 at 04:18:02PM +0100, Ganael Laplanche wrote:
> Digging into a problem with a project of mine [1], I found dash (0.5.12) 
> behaves differently from other POSIX-compliant shells when dealing with 
> monitor mode when running in the background.

> In the following example, dash immediately sends a SIGTTIN signal that stops 
> the process group:

> $ cat test.sh
> set -m
> $ dash test.sh &
> [1]  + suspended (tty input)  dash test.sh

> while other shells do not:

> $ bash test.sh &
> [1]  + done       bash test.sh
> $ sh test.sh & # (/bin/sh from FreeBSD 13.1-RELEASE)
> [1]  + done       sh test.sh
> $ ksh93 test.sh &
> [1]  + done       ksh93 test.sh

> Is that an intended behaviour ?

The loop with SIGTTIN is the right thing for an interactive job control
shell. It ensures that two job control shells do not interfere. The idea
is that the user can later 'fg' from the outer job control shell.

The loop also helps with suspending a job control shell, for example via
the funcs/suspend function from the ash source (containing 'local -',
'set +m' and 'kill -TSTP 0'): when trying to resume the inner shell via
'bg' in the outer shell, it is this loop that makes the inner shell stop
itself.

However, for a non-interactive shell, monitor mode is most useful for
its effect of placing jobs in their own process groups. This does not
necessarily imply any tty manipulation.

To make this possible, feature was added to FreeBSD sh in
https://cgit.freebsd.org/src/commit/?id=cd60e2c67d52e1f957841af19128c7227880743a

This commit allows using job control without a tty in non-interactive
mode. Where an interactive shell disables job control with the message
  can't access tty; job control turned off
or stops itself because it is not in the foreground, a non-interactive
shell instead leaves a limited form of job control enabled that only
sets process group IDs on new processes and does not alter the
terminal's foreground process group ID.

> Also, the following test leads to dash looping indefinitely and eating 100% 
> CPU:

> $ cat test2.sh
> trap '' 21
> set -m
> $ dash test2.sh &

> It remains stuck within the following loop:

> https://git.kernel.org/pub/scm/utils/dash/dash.git/tree/src/jobs.c?
> h=v0.5.12#n208

> Weird... Is that a dash bug or am I missing something?

Manipulating the disposition of TTIN/TTOU/TSTP with job control enabled
might be a question of "if it hurts, don't do that".

-- 
Jilles Tjoelker

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Tuesday, January 10, 2023 10:28:42 PM CET Jilles Tjoelker wrote:

Hello Jilles,

> The loop with SIGTTIN is the right thing for an interactive job control
> shell. It ensures that two job control shells do not interfere. The idea
> is that the user can later 'fg' from the outer job control shell.
> 
> The loop also helps with suspending a job control shell, for example via
> the funcs/suspend function from the ash source (containing 'local -',
> 'set +m' and 'kill -TSTP 0'): when trying to resume the inner shell via
> 'bg' in the outer shell, it is this loop that makes the inner shell stop
> itself.

Thanks for those explanations.

> However, for a non-interactive shell, monitor mode is most useful for
> its effect of placing jobs in their own process groups. This does not
> necessarily imply any tty manipulation.
> 
> To make this possible, feature was added to FreeBSD sh in
> https://cgit.freebsd.org/src/commit/?id=cd60e2c67d52e1f957841af19128c7227880
> 743a
> 
> This commit allows using job control without a tty in non-interactive
> mode.
> [...]

This is exactly what I use fpsync for (to get a new process group). Do I 
understand that this feature is not available in Dash ?

> Manipulating the disposition of TTIN/TTOU/TSTP with job control enabled
> might be a question of "if it hurts, don't do that".

Thanks!

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Wednesday, January 11, 2023 5:25:10 PM CET Ganael Laplanche wrote:

> This is exactly what I use fpsync for (to get a new process group). Do I
> understand that this feature is not available in Dash ?

I'll rephrase : this is exactly what I use *for* fpsync (to get a new process 
group for each sub-job). Do I understand that this feature is not available in 
Dash ?

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

Ganael Laplanche wrote in
 <3939408.BRNeRiNLvY@home.martymac.org>:
 |On Wednesday, January 11, 2023 5:25:10 PM CET Ganael Laplanche wrote:
 |
 |> This is exactly what I use fpsync for (to get a new process group). Do I
 |> understand that this feature is not available in Dash ?
 |
 |I'll rephrase : this is exactly what I use *for* fpsync (to get a new \
 |process 
 |group for each sub-job). Do I understand that this feature is not availa\
 |ble in 
 |Dash ?

Just to note that shells may have problems with monitor mode
(especially scripted, non-interactive).  This was a problem when
writing a scaling multi-processed test script.  I now do

        [ -n "${JOBMON}" ] && set -m >/dev/null 2>&1
        ( # Place the job in its own directory to ease file management
                trap '' EXIT HUP INT QUIT TERM USR1 USR2
                JOB_MSG_ID=
                ${mkdir} t.${JOBS}.d && cd t.${JOBS}.d &&
                        eval t_${1} ${JOBS} ${1} &&
                        ${rm} -f ../t.${JOBS}.id
        ) > t.${JOBS}.io </dev/null & # 2>&1 </dev/null &
        i=${!}
        [ -n "${JOBMON}" ] && set +m >/dev/null 2>&1
        JOBLIST="${JOBLIST} ${i}"

This is a problem if $JOBMON=, as unfortunately it may leave
zombie processes running.  Luckily all {,/usr/xpg4}/bin/sh'ells do
this well, including dash.

--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)

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Wednesday, January 11, 2023 6:31:50 PM CET Steffen Nurpmeso wrote:

Hello Steffen,

> Just to note that shells may have problems with monitor mode
> (especially scripted, non-interactive).  This was a problem when
> writing a scaling multi-processed test script.  I now do
> [...]

Thanks for your feedback.

I use the same kind of code in fpsync and it works great with dash, right.

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 ?

Best regards,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

Hello.

Ganael Laplanche wrote in
 <2273710.GFeA0BcqPk@dmc12.centralesupelec.fr>:
 |On Wednesday, January 11, 2023 6:31:50 PM CET Steffen Nurpmeso wrote:
 |> Just to note that shells may have problems with monitor mode
 |> (especially scripted, non-interactive).  This was a problem when
 |> writing a scaling multi-processed test script.  I now do
 |> [...]
 |
 |Thanks for your feedback.
 |
 |I use the same kind of code in fpsync and it works great with dash, right.
 |
 |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 ?

It seems i have not really looked at the problem you have.
Sorry, yes, that not.

--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)

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Thursday, January 12, 2023 4:00:49 PM CET Steffen Nurpmeso wrote:

Hello Steffen,

>  |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 ?
> 
> It seems i have not really looked at the problem you have.
> Sorry, yes, that not.

Thanks for this confirmation!

Best regards,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

Hello Ganael.

Ganael Laplanche wrote in
 <2755634.0OmqyTZxqa@dmc12.centralesupelec.fr>:
 |On Thursday, January 12, 2023 4:00:49 PM CET Steffen Nurpmeso wrote:
 |
 |>|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 ?
 |> 
 |> It seems i have not really looked at the problem you have.
 |> Sorry, yes, that not.
 |
 |Thanks for this confirmation!

Well i mean, in spirit of Wheeler's "anything is possible with
(one more) indirection" it could be possible nonetheless?

  #!/bin/dash
  rm -f BGBG
  (
  set -m
  i=$(set -o)
  echo >&2 set sub 1 is $i
  (
  sleep 1
  echo i am here > BGBG
  set -o >> BGBG
  ) &
  i=$!
  echo >&2 JOB IS $i
  wait $i
  ) & #</dev/null >/dev/null &

--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)

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Friday, January 13, 2023 10:03:35 PM CET Steffen Nurpmeso wrote:

Hello Steffen,

> Well i mean, in spirit of Wheeler's "anything is possible with
> (one more) indirection" it could be possible nonetheless?
> [...]

Well, my initial examples then become :

$ cat test_ko.sh
set -m
$ dash test_ko.sh &
[1]  + suspended (tty input)  dash test_ko.sh

$ cat test_ok.sh
(
  set -m
)
$ dash test_ok.sh &
[1]  + done       dash test_ok.sh

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:

$ cat test_pgid.sh
(
  set -m
  (
    sleep 1
  ) &
  i=$!
  echo "Main shell has: $(ps -o pid,pgid $$)"
  echo "Sub-shell has: $(ps -o pid,pgid $i)"
  wait $i
)
$ dash test_pgid.sh &
Main shell has:   PID  PGID
10362 10362
Sub-shell has:   PID  PGID
10364 10362
[1]  + done       dash test_pgid.sh

So I am afraid the 'extra fork' tip is not useable for me...

Cheers,

Ganael.

[1] Discussed here: https://lists.freebsd.org/archives/freebsd-hackers/2022-March/000924.html

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

Hello Ganael.

Ganael Laplanche wrote in
 <2616938.TYJnH3iKXO@home.martymac.org>:
 |On Friday, January 13, 2023 10:03:35 PM CET Steffen Nurpmeso wrote:
 |> Well i mean, in spirit of Wheeler's "anything is possible with
 |> (one more) indirection" it could be possible nonetheless?
 ...
 |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 requirem\
 |ent. 
 |The following example shows that the new process' PGID remains the \
 |same as the 
 |initial shell:
 |
 |$ cat test_pgid.sh
 |(
 |  set -m
 |  (
 |    sleep 1
 |  ) &
 |  i=$!
 |  echo "Main shell has: $(ps -o pid,pgid $$)"
 |  echo "Sub-shell has: $(ps -o pid,pgid $i)"
 |  wait $i
 |)
 |$ dash test_pgid.sh &
 |Main shell has:   PID  PGID
 |10362 10362
 |Sub-shell has:   PID  PGID
 |10364 10362
 |[1]  + done       dash test_pgid.sh
 |
 |So I am afraid the 'extra fork' tip is not useable for me...

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.

  ...
 |[1] Discussed here: https://lists.freebsd.org/archives/freebsd-hackers/2\
 |022-March/000924.html

Very interesting.  With the correction (fwiw) that

  * with bash, it hangs (as expected with a new process group)

is not true with bash 5.2.15 on Linux 6.1 with glibc

  $ bash -c '(set -m; echo "echo from background" & wait "$!"); echo "wait returns $?"'
  echo from background
  [1]+  Done                    echo "echo from background"
  wait returns 0

(Well the following "hangs" in that it does not print a prompt)

  $ bash -c '(set -m; echo "echo from background $$" & echo $!; wait "$!"); echo "wait returns $?"' &
  [1] 21123
  #?0|kent:nail.git$ 21125
  echo from background 21123
  [1]+  Done                    echo "echo from background $$"
  wait returns 0

So i think i am out of ideas except doing what Jilles suggested
in the message, enwrapping the inner thing with sh -c '..'.
And that seems to work a bit as

  #!/bin/dash
  #(
    sh -c '
    set -m
    (
      sleep 1
    ) &
    i=$!
    echo >&2 "inner Main shell has: $(ps -o pid,pgid $$)"
    echo >&2 "inner Sub-shell has: $(ps -o pid,pgid $i)"
    wait $i
    ' </dev/null >/dev/null &
    echo "Main shell has: $(ps -o pid,pgid $$)"
  #)
    echo "outer Main shell has: $(ps -o pid,pgid $$)"

ends up as

  #?148|kent:tmp$ ./t.sh
  Main shell has:   PID  PGID
  22300 22300
  inner Main shell has:   PID  PGID
  22301 22301
  outer Main shell has:   PID  PGID
  22300 22300
  #?0|kent:tmp$ inner Sub-shell has:   PID  PGID
  22303 22303
  sh: 9: Cannot set tty process group (No such process)

  logout
  There are stopped jobs.

Ciao!

--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)

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Monday, January 16, 2023 8:41:14 PM CET Steffen Nurpmeso wrote:

Hello Steffen,

> So i think i am out of ideas except doing what Jilles suggested
> in the message, enwrapping the inner thing with sh -c '..'.
> [...]

Yes, I am afraid it's the only syntax that works (but it is not very 
convenient).

I had to remove /dev/null redirections for stdin and stdout to avoid getting a 
"Cannot set tty process group (Operation not permitted)" error.

Finally, that gives, on FreeBSD:

$ cat ./test.sh
#!/usr/local/bin/dash
/usr/local/bin/dash -c '
    set -m
    (
      sleep 1
    ) &
    i=$!
    echo >&2 "inner Main shell has: $(ps -o pid,pgid $$)"
    echo >&2 "inner Sub-shell has: $(ps -o pid,pgid $i)"
    wait $i
'
echo "outer Main shell has: $(ps -o pid,pgid $$)"

$ ./test.sh
inner Main shell has:   PID  PGID
20457 20457
inner Sub-shell has:   PID  PGID
20458 20458
outer Main shell has:   PID  PGID
20456 20456

Thanks again for your help!

Cheers,

Ganael.

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

Hello Ganael.

For nothing really!

Ganael Laplanche wrote in
 <2021198.x0N0T6uNKo@home.martymac.org>:
 |On Monday, January 16, 2023 8:41:14 PM CET Steffen Nurpmeso wrote:
 |> So i think i am out of ideas except doing what Jilles suggested
 |> in the message, enwrapping the inner thing with sh -c '..'.
 |> [...]
 |
 |Yes, I am afraid it's the only syntax that works (but it is not very 
 |convenient).
 |
 |I had to remove /dev/null redirections for stdin and stdout to avoid \
 |getting a 
 |"Cannot set tty process group (Operation not permitted)" error.

In the meantime i have taken over Jilles' commit to busybox ash,
hopefully correct, let's see whether it is taken.

 |Finally, that gives, on FreeBSD:
 |
 |$ cat ./test.sh
 |#!/usr/local/bin/dash
 |/usr/local/bin/dash -c '
 |    set -m
 |    (
 |      sleep 1
 |    ) &
 |    i=$!

I could avoid many problems by doing "set +m" here.  But not all.

 |    echo >&2 "inner Main shell has: $(ps -o pid,pgid $$)"
 |    echo >&2 "inner Sub-shell has: $(ps -o pid,pgid $i)"
 |    wait $i
 |'
 |echo "outer Main shell has: $(ps -o pid,pgid $$)"
 |
 |$ ./test.sh
 |inner Main shell has:   PID  PGID
 |20457 20457
 |inner Sub-shell has:   PID  PGID
 |20458 20458
 |outer Main shell has:   PID  PGID
 |20456 20456
 |
 |Thanks again for your help!

Ah, all on my side.  For example i find it disturbing that my
interactive shell (bash) exits when the script backgrounds the
recursive shell above via & .. it reads EOF, and without "set -o
ignoreeof" or jobbed job it goes home.  I would not have expected
this.  (If is start it via "./test.sh &" then ok, too.)
No expert, but at occasions Wheeler really helps out.

Ciao,

--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)

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Tuesday, January 17, 2023 10:04:01 PM CET Steffen Nurpmeso wrote:

Hello Steffen,

> In the meantime i have taken over Jilles' commit to busybox ash,
> hopefully correct, let's see whether it is taken.

Fantastic, thanks!

Any chance to get it backported to Dash too ?

> [...]
> Ah, all on my side.  For example i find it disturbing that my
> interactive shell (bash) exits when the script backgrounds the
> recursive shell above via & .. it reads EOF, and without "set -o
> ignoreeof" or jobbed job it goes home.  I would not have expected
> this.  (If is start it via "./test.sh &" then ok, too.)
> No expert, but at occasions Wheeler really helps out.

Cheers,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Wednesday, January 18, 2023 12:40:12 PM CET Ganael Laplanche wrote:

Hello again,

Oops, my previous example was missing the final ampersand.

For the recall, here is what actually works (on FreeBSD, including the set +m 
fix):

$ cat ./test.sh
#!/usr/local/bin/dash
/usr/local/bin/dash -c '
    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
' &
echo "outer Main shell has: $(ps -o pid,pgid $$)"

$ ./test.sh
outer Main shell has:   PID  PGID
47919 47919
inner Main shell has:   PID  PGID                                                                                                                                                                                  
47920 47919
inner Sub-shell has:   PID  PGID
47922 47922

Cheers,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

Hello Ganael.

Ganael Laplanche wrote in
 <2175334.ZhQv657J3T@dmc12.centralesupelec.fr>:
 |On Tuesday, January 17, 2023 10:04:01 PM CET Steffen Nurpmeso wrote:
 |> In the meantime i have taken over Jilles' commit to busybox ash,
 |> hopefully correct, let's see whether it is taken.
 |
 |Fantastic, thanks!
 |
 |Any chance to get it backported to Dash too ?

Not a "backport", but from shallow testing (i have no idea what
i am doing), the attached patch works.

Ciao,

--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)

[-- Attachment #2: dash.patch --]
[-- Type: text/x-diff, Size: 5446 bytes --]

From 39aa73889163629aa477adf536b557b80faa2bfc Mon Sep 17 00:00:00 2001
Message-Id: <39aa73889163629aa477adf536b557b80faa2bfc.1674083695.git.steffen@sdaoden.eu>
From: Steffen Nurpmeso <steffen@sdaoden.eu>
Date: Thu, 19 Jan 2023 00:14:22 +0100
Subject: [PATCH] Allow enabling job control without a tty in non-interactive
 mode..

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.
---
 src/jobs.c | 125 +++++++++++++++++++++++++++++++++--------------------
 1 file changed, 78 insertions(+), 47 deletions(-)

diff --git a/src/jobs.c b/src/jobs.c
index f3b9ffc285..878cc4f7c0 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,95 @@ 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 = open(_PATH_TTY, O_RDWR | O_CLOEXEC)) < 0) {
+			i = 0;
+			while (i <= 2 && !isatty(i))
+				i++;
+			if (i > 2 ||
+			    (ttyfd = fcntl(i, F_DUPFD_CLOEXEC, 10)) < 0) {
+				jobctl_notty();
+				return;
+			}
+		}
+		if (ttyfd < 10) {
+			/*
+			 * Keep our TTY file descriptor out of the way of
+			 * the user's redirections.
+			 */
+			if ((i = fcntl(ttyfd, F_DUPFD_CLOEXEC, 10)) < 0) {
+				jobctl_notty();
+				return;
+			}
+			close(ttyfd);
+			ttyfd = i;
 		}
-		fd = savefd(fd, ofd);
 		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 +419,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 +482,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 +906,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 +1046,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 +1536,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)
-- 
2.39.0

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Thursday, January 19, 2023 12:15:34 AM CET Steffen Nurpmeso wrote:

Hello Steffen,

> Not a "backport", but from shallow testing (i have no idea what
> i am doing), the attached patch works.

Yes, it works ! With that patch, fpsync runs smoothly in the background (with 
specific PGIDs for sub-jobs) !

Could a Dash developer review (and, if possible, commit) it ?

Again, Steffen, thank you very much for your help :)

Cheers,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

Hello Ganael.

Ganael Laplanche wrote in
 <2024648.dncibcYME7@dmc12.centralesupelec.fr>:
 |On Thursday, January 19, 2023 12:15:34 AM CET Steffen Nurpmeso wrote:
 |> Not a "backport", but from shallow testing (i have no idea what
 |> i am doing), the attached patch works.
 |
 |Yes, it works ! With that patch, fpsync runs smoothly in the background \
 |(with 
 |specific PGIDs for sub-jobs) !
 |
 |Could a Dash developer review (and, if possible, commit) it ?

Yes, replace the open() with a sh_open(_PATH_TTY, O_RDWR, 1), and
use savefd() not dupfd without cloexec awareness.  Was a five
minute thing actually, the busybox patch was right i think.

Hm, i will attach a better one..  Shall anyone consider this,
please use that not the older one!  Thanks.

 |Again, Steffen, thank you very much for your help :)

Thanks to Jilles.  But walked.
(In fact .. i had the impression that someone should review
compilation without !JOBS.  Or, maybe, drop the option.)

Ciao.

--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)

[-- Attachment #2: dash.patch --]
[-- Type: text/x-diff, Size: 5207 bytes --]

From dedaa3fa370ea9c4aeb1771b5568a7bef4065b04 Mon Sep 17 00:00:00 2001
Message-Id: <dedaa3fa370ea9c4aeb1771b5568a7bef4065b04.1674153195.git.steffen@sdaoden.eu>
From: Steffen Nurpmeso <steffen@sdaoden.eu>
Date: Thu, 19 Jan 2023 19:32:55 +0100
Subject: [PATCH] Allow enabling job control without a tty in non-interactive
 mode..

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.
---
 src/jobs.c | 114 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 67 insertions(+), 47 deletions(-)

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)
-- 
2.39.0

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Thursday, January 19, 2023 7:33:30 PM CET Steffen Nurpmeso wrote:

Hello Steffen,

> Hm, i will attach a better one..  Shall anyone consider this,
> please use that not the older one!  Thanks.

Thanks for the update!

I can confirm that your new patch:

- allows to run fpsync in the background
- fixes the situation with the loop using 100% CPU
  (detailed at the beginning of that thread)

I hope someone from dash team will have a look at it :)

Cheers,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Herbert Xu]

Steffen Nurpmeso <steffen@sdaoden.eu> wrote:
.
> Hm, i will attach a better one..  Shall anyone consider this,
> please use that not the older one!  Thanks.

Could you please resubmit the patch inline? For some reason it's
not getting picked up by patchwork and the quoted-printable format
does not a bit iffy.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Friday, January 27, 2023 11:21:26 AM CET Herbert Xu wrote:

Hello Herbert,

> Could you please resubmit the patch inline? For some reason it's
> not getting picked up by patchwork and the quoted-printable format
> does not a bit iffy.

Here it is:

8<----
From dedaa3fa370ea9c4aeb1771b5568a7bef4065b04 Mon Sep 17 00:00:00 2001
Message-Id: <dedaa3fa370ea9c4aeb1771b5568a7bef4065b04.1674153195.git.steffen@sdaoden.eu>
From: Steffen Nurpmeso <steffen@sdaoden.eu>
Date: Thu, 19 Jan 2023 19:32:55 +0100
Subject: [PATCH] Allow enabling job control without a tty in non-interactive
 mode..

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.
---
 src/jobs.c | 114 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 67 insertions(+), 47 deletions(-)

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)
-- 
2.39.0
8<----

Thanks for your help!

Best regards,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Herbert Xu]

On Fri, Jan 27, 2023 at 12:46:35PM +0100, Ganael Laplanche wrote:
>
> Here it is:

Sorry but that didn't work either, please have a look at the
list of patches on the queue:

https://patchwork.kernel.org/project/dash/list/

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Monday, January 30, 2023 8:21:52 AM CET Herbert Xu wrote:

Hello Herbert,

> Sorry but that didn't work either, please have a look at the
> list of patches on the queue:
> 
> https://patchwork.kernel.org/project/dash/list/

Well, it seems the previous patch had space problems.

Find a fixed version attached.

Best regards,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

[-- Attachment #2: dash.patch --]
[-- Type: text/x-patch, Size: 4888 bytes --]

From dedaa3fa370ea9c4aeb1771b5568a7bef4065b04 Mon Sep 17 00:00:00 2001
Message-Id: <dedaa3fa370ea9c4aeb1771b5568a7bef4065b04.1674153195.git.steffen@sdaoden.eu>
From: Steffen Nurpmeso <steffen@sdaoden.eu>
Date: Thu, 19 Jan 2023 19:32:55 +0100
Subject: [PATCH] Allow enabling job control without a tty in non-interactive
 mode..

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.
---
 src/jobs.c | 114 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 67 insertions(+), 47 deletions(-)

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 @@
 
 #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 @@
 	}
 }
 
-#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 @@
 	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 @@
 	int indent;
 	char s[80];
 
+	if (!iflag)
+		return;
+
 	ps = jp->ps;
 
 	if (mode & SHOW_PGID) {
@@ -878,7 +895,7 @@
 		/* 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 @@
 	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 @@
 
 #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)

Re: Monitor mode handling (bug ?)

[Herbert Xu]

On Mon, Jan 30, 2023 at 12:37:28PM +0100, Ganael Laplanche wrote:
> 
> Well, it seems the previous patch had space problems.
> 
> Find a fixed version attached.

Thanks! This seems to have made it into patchwork.  I will review
it.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Tuesday, January 31, 2023 4:20:25 AM CET Herbert Xu wrote:

> Thanks! This seems to have made it into patchwork.  I will review
> it.

Perfect, thanks Herbert!

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Tuesday, January 31, 2023 12:46:57 PM CET Ganael Laplanche wrote:

Hello Herbert,

I hope you are doing fine.

> > Thanks! This seems to have made it into patchwork.  I will review
> > it.
> 
> Perfect, thanks Herbert!

It seems the patch has been archived in Patchwork :

https://patchwork.kernel.org/project/dash/patch/
dedaa3fa370ea9c4aeb1771b5568a7bef4065b04.1675113321.git.steffen@sdaoden.eu/

Is that Intended ? Has it been refused ?

Cheers,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Steffen Nurpmeso]

Ganael Laplanche wrote in
 <1742108.iAzbLhWyP0@dmc12.centralesupelec.fr>:
 |On Tuesday, January 31, 2023 12:46:57 PM CET Ganael Laplanche wrote:
 |Hello Herbert,
 |
 |I hope you are doing fine.
 |
 |>> Thanks! This seems to have made it into patchwork.  I will review
 |>> it.
 |> 
 |> Perfect, thanks Herbert!
 |
 |It seems the patch has been archived in Patchwork :
 |
 |https://patchwork.kernel.org/project/dash/patch/
 |dedaa3fa370ea9c4aeb1771b5568a7bef4065b04.1675113321.git.steffen@sdaoden.eu/
 |
 |Is that Intended ? Has it been refused ?

I would not worry that soon; some examples from git:

  AuthorDate: 2022-01-19 16:37:54 +1100
  CommitDate: 2022-12-07 16:27:49 +0800
...
  AuthorDate: 2021-09-05 09:36:11 +0100
  CommitDate: 2022-12-07 16:27:49 +0800

So maybe .. Easter 2024 .. seems an appropriate ping date.
:-)

--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)

Re: Monitor mode handling (bug ?)

[Herbert Xu]

On Wed, Mar 08, 2023 at 12:47:23PM +0100, Ganael Laplanche wrote:
>
> It seems the patch has been archived in Patchwork :
> 
> https://patchwork.kernel.org/project/dash/patch/
> dedaa3fa370ea9c4aeb1771b5568a7bef4065b04.1675113321.git.steffen@sdaoden.eu/
> 
> Is that Intended ? Has it been refused ?

Thanks for letting me know! I have no idea how this happened.

I did a search and discovered quite a few archived patches.
I've unarcived all of them.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Thursday, March 9, 2023 6:55:26 AM CET Herbert Xu wrote:

Hello Herbert,

> Thanks for letting me know! I have no idea how this happened.
> 
> I did a search and discovered quite a few archived patches.
> I've unarcived all of them.

Thanks, but it still appears as archived ("State: New, archived") and does not 
show up in :

https://patchwork.kernel.org/project/dash/list/

It seems it has been flagged as "New" but the old "archived" flag is still 
there...

Best regards,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org

Re: Monitor mode handling (bug ?)

[Herbert Xu]

On Thu, Mar 09, 2023 at 07:40:42AM +0100, Ganael Laplanche wrote:
>
> Thanks, but it still appears as archived ("State: New, archived") and does not 
> show up in :
> 
> https://patchwork.kernel.org/project/dash/list/
> 
> It seems it has been flagged as "New" but the old "archived" flag is still 
> there...

Nasty, something is automatically archiving all the patches.

I'll try to find out why.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Monitor mode handling (bug ?)

[Herbert Xu]

On Thu, Mar 09, 2023 at 05:22:23PM +0800, Herbert Xu wrote:
>
> Nasty, something is automatically archiving all the patches.
> 
> I'll try to find out why.

I've changed all the patches to "Under Review" and hopefully
that should keep them from being moved back into the archive.

If you notice anything strange please let me know.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Monitor mode handling (bug ?)

[Ganael Laplanche]

On Thursday, March 9, 2023 10:55:51 AM CET Herbert Xu wrote:

> I've changed all the patches to "Under Review" and hopefully
> that should keep them from being moved back into the archive.
> 
> If you notice anything strange please let me know.

Perfect, thanks!

Have a nice day,

-- 
Ganael LAPLANCHE <ganael.laplanche@martymac.org>
http://www.martymac.org | http://contribs.martymac.org
FreeBSD: martymac <martymac@FreeBSD.org>, http://www.FreeBSD.org