Re: bash passes changed termios to backgrounded process(es) groups?

[Steffen Nurpmeso]

Chet Ramey wrote in
 <b1ae5f45-a9fe-407f-8a76-6e36e7248166@case.edu>:
 |On 8/28/24 4:12 PM, Steffen Nurpmeso wrote:
 |> Chet Ramey wrote in
 |>   <3ca901aa-5c5e-4be3-9a71-157d7101f892@case.edu>:
 |>|On 8/27/24 7:46 PM, Steffen Nurpmeso wrote:
 |>|> I got a bug report for my mailer which stated
 |>|>
 |>|>     $ ( echo blah | Mail root ) &
 |>|>    [1] 2754649
 |>|>     $ ^M^M^M^M^C^C
 |>|>
 |>|>    [1]+  Stopped                 ( echo blah | Mail root )
 |>|>     $ fg
 |>|>    ( echo blah | Mail root )
 |>|>     $
 |>|>
 |>|> I turns out i answered him now
 ...
 |>|> ..and it seems that if bash starts a normal process then ICRNL is
 |>|> set, but if it starts a (process)& or only process&, then not!
 |>|> (I was about to send this to bug-readline first.)
 |>|
 |>|Under no circumstances should a background process attempt to fetch or
 |>|modify terminal attributes. Why isn't your Mail process checking \
 |>|for that?
 |> 
 |> How could it do so?
 |> (getpid()==tcgetpgrp() or what the function name is is the only
 |> idea i have, but note it is false for (EXE), too.  *Big problem*!)
 |
 |It's not a big problem. You're in the background if your process group is
 |not equal to the terminal's process group. A simple test suffices:
 |
 |tty = fileno (stderr);        /* pick your file descriptor */
 |pgrp = getpgid(0);
 |tpgrp = tcgetpgrp (tty);
 |running_in_background = (pgrp != tpgrp);
 |
 |>|Chances are excellent that it will fetch the terminal attributes \
 |>|as they've
 |>|been set by readline when it goes to read the next command, then \
 |>|modify (?)
 |>|them out from underneath readline.
 |> 
 |> Yes, it is not right what readline does there.
 |
 |No, readline does the right thing. It fetches and sets the terminal
 |attributes while it has control, and it relies on the caller to make
 |sure it's in the right process group (that is, it doesn't try to set
 |the process group itself). It tries to set the window size immediately --
 |before fetching the terminal attributes -- so it gets a SIGTTOU if it's
 |in the background, even though that can be defeated by `stty -tostop'.
 |It restores the terminal attributes before it returns a value to its
 |caller.

Yes i apologise for this accusation.  The bug(s are) is solely on
my part.

 |> And for me, two things.  For one we ensure we give to child
 |> processes, and to restore whenever we go, the original settings as
 |> we inherit them. 
 |
 |If you're in the background, you have no idea what the foreground process
 |group has done to the terminal settings before you query them. You only
 |fetch and modify the terminal settings if you're in the foreground.

Inherently racy, yes.
Yes, we need to wait if we are running in the background
interactively.

 |> We requery what means "original" whenever we get
 |> back from a suspension, because user etc may apply changes, and we
 |> should reflect (i have seen that, or even got a bug report).
 |
 |Sure. But if you are restarted (and get your SIGCONT) due to the equivalent
 |of a `bg', you still have to check whether you're in the foreground.

That is an interesting thought i will soon look at.  Thanks.

 |> And second, if there isatty(3) somewhere, we do termios stuff;
 |> i agree this is bad, especially so since we look STDIN and STDOUT,
 |> not STDIN and STDERR, as POSIX says a shell should do (i think
 |> even dash that was notoriously "wrong" with that will have fixed
 |> this with the next release). 
 |
 |You're not a shell, but looking at stderr is probably the right thing.

Tja.  Not yet, that.

 |> "Interactive" we are only if both
 |> are isatty(3), and maybe i will change all that because we are no
 |> shell and never will be, to only be interactive and do termios
 |> stuff if all relevant FDs are terminals.
 |
 |Also probably the right thing.

Totally off-topic for bug-bash in any case.

 |> (Anyhow, in the example we start a child process, and since STDERR
 |> is passed "as is", we try to restore termions settings (which,
 |> btw, have never been changed in the above snippet, but that
 |> aside), *if* i remember the context correctly.  Our termios code
 |> is a stack, and it is terribly complicated.)
 |
 |Still the wrong thing to do if you're in the background. It's an
 |inherent race condition.

Well it generates TTOU.  I mean, it could very well be we ask for
a password in this "interactive" situation, dependent on the
configuration, you normally do not run this thing backgrounded.
I never did so far (like so).
This is unfortunately very badly documented in our manual (ie
i told him he should redirect STDOUT to /dev/null to get where he
effectively wants to be; apart from the fact he likely only was so
nice to generate a bug report).

Thanks -- and apologies for this false accusation.
Of course it is a tremendous concurrent messy race against
something we have to keep our fingers off.

 --End of <b1ae5f45-a9fe-407f-8a76-6e36e7248166@case.edu>

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