From: Herbert Xu <herbert@gondor.apana.org.au>
To: Harald van Dijk <harald@gigawatt.nl>
Cc: steffen@sdaoden.eu, dash@vger.kernel.org,
vda.linux@googlemail.com, jilles@stack.nl
Subject: [PATCH] jobs: Block signals during tcsetpgrp
Date: Wed, 6 Jan 2021 15:45:12 +1100 [thread overview]
Message-ID: <20210106044512.GA28191@gondor.apana.org.au> (raw)
In-Reply-To: <9bb6fbb5-20e1-eae1-0144-67a4c7e20496@gigawatt.nl>
Harald van Dijk <harald@gigawatt.nl> wrote:
> On 19/12/2020 22:21, Steffen Nurpmeso wrote:
>> Steffen Nurpmeso wrote in
>> <20201219172838.1B-WB%steffen@sdaoden.eu>:
>> |Long story short, after falsely accusing BSD make of not working
>>
>> After dinner i shortened it a bit more, and attach it again, ok?
>> It is terrible, but now less redundant than before.
>> Sorry for being so terse, that problem crosses my head for about
>> a week, and i was totally mislead and if you bang your head
>> against the wall so many hours bugs or misbehaviours in a handful
>> of other programs is not the expected outcome.
>
> I think a minimal test case is simply
>
> all:
> $(SHELL) -c 'trap "echo TTOU" TTOU; set -m; echo all good'
>
> unless I accidentally oversimplified.
>
> The SIGTTOU is caused by setjobctl's xtcsetpgrp(fd, pgrp) call to make
> its newly started process group the foreground process group when job
> control is enabled, where xtcsetpgrp is a wrapper for tcsetpgrp. (That's
> in dash, the other variants may have some small differences.) tcsetpgrp
> has this little bit in its specification:
>
> Attempts to use tcsetpgrp() from a process which is a member of
> a background process group on a fildes associated with its con‐
> trolling terminal shall cause the process group to be sent a
> SIGTTOU signal. If the calling thread is blocking SIGTTOU sig‐
> nals or the process is ignoring SIGTTOU signals, the process
> shall be allowed to perform the operation, and no signal is
> sent.
>
> Ordinarily, when job control is enabled, SIGTTOU is ignored. However,
> when a trap action is specified for SIGTTOU, the signal is not ignored,
> and there is no blocking in place either, so the tcsetpgrp() call is not
> allowed.
>
> The lowest impact change to make here, the one that otherwise preserves
> the existing shell behaviour, is to block signals before calling
> tcsetpgrp and unblocking them afterwards. This ensures SIGTTOU does not
> get raised here, but also ensures that if SIGTTOU is sent to the shell
> for another reason, there is no window where it gets silently ignored.
>
> Another way to fix this is by not trying to make the shell start a new
> process group, or at least not make it the foreground process group.
> Most other shells appear to not try to do this.
This patch implements the blocking of SIGTTOU (and everything else)
while we call tcsetpgrp.
Reported-by: Steffen Nurpmeso <steffen@sdaoden.eu>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/src/jobs.c b/src/jobs.c
index 516786f..809f37c 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -1512,7 +1512,13 @@ showpipe(struct job *jp, struct output *out)
STATIC void
xtcsetpgrp(int fd, pid_t pgrp)
{
- if (tcsetpgrp(fd, pgrp))
+ int err;
+
+ sigblockall(NULL);
+ err = tcsetpgrp(fd, pgrp);
+ sigclearmask();
+
+ if (err)
sh_error("Cannot set tty process group (%s)", strerror(errno));
}
#endif
--
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
next prev parent reply other threads:[~2021-01-06 4:46 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-19 17:28 dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think Steffen Nurpmeso
2020-12-19 22:21 ` Steffen Nurpmeso
2020-12-19 23:52 ` Harald van Dijk
2020-12-21 16:24 ` Jilles Tjoelker
2020-12-21 19:43 ` Steffen Nurpmeso
2020-12-23 20:18 ` Harald van Dijk
2020-12-24 15:29 ` Jilles Tjoelker
2021-01-10 23:56 ` Harald van Dijk
2021-01-06 4:46 ` Herbert Xu
2021-01-06 4:45 ` Herbert Xu [this message]
2021-01-06 21:16 ` [PATCH] jobs: Block signals during tcsetpgrp Harald van Dijk
2021-01-06 22:41 ` Jilles Tjoelker
2021-01-07 7:36 ` Denys Vlasenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210106044512.GA28191@gondor.apana.org.au \
--to=herbert@gondor.apana.org.au \
--cc=dash@vger.kernel.org \
--cc=harald@gigawatt.nl \
--cc=jilles@stack.nl \
--cc=steffen@sdaoden.eu \
--cc=vda.linux@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).