poll()/select() on a tty fd in a backgrounded process raises SIGTTIN
Jon Turney
jon.turney@dronecode.org.uk
Tue Jul 26 21:51:00 GMT 2016
On 25/07/2016 17:55, Corinna Vinschen wrote:
> On Jul 25 17:52, Jon Turney wrote:
>>>> This test case is reduced from a problem seen with gdb, since commit
>>>> 0b333c5e, where gdb stops with SIGTTIN when the inferior is started.
>>>
>>> Did you find out where it stops and do you have any ideas how we could
>>> fix this?
>>
>> Um.
>>
>> gdb stops because a poll() on stdin (even when stdin is not ready to read)
>> while backgrounded causes a SIGTTIN to be raised.
>>
>> I think this caused by select.cc:peek_console/peek_pipe calling
>> fhandler_termios::bg_check(SIGTTIN).
>>
>> My idea for fixing it is to remove that behaviour from cygwin, assuming I'm
>> right in thinking that the behaviour is incorrect, although I'm wary of
>> touching something that's so old, and I don't understand why that call to
>> bg_check is in there at all...
So, this is presumably so that the exception conditions which bg_check()
knows about make the fd ready, irrespective of if peeking it shows some
input.
> This may be an (old) misunderstanding how that's supposed to work.
>
> Just removing sounds a bit dangerous, but we can try it. Maybe it
> just isn't necessary, but we need to check if it doesn't break
> something else. Like, say, not stopping at all anymore...
Attached is the patch as discussed on IRC, with some additional commentary.
After writing these comments, it's perhaps a bit clearer to me that
bg_check() could be re-written to take a bg_check_operation enum (read,
write, change_settings, peek), rather than trying to encode that
operation as a signal number, but that might be changing things too much.
Also, looking at the uses of bg_check(), it's odd that
handler_console::ioctl for TIOCSWINSZ calls bg_check (SIGTTOU), which
should probably be -SIGTTOU, otherwise a background process could resize
a console window without causing SIGTTOU. But then that operation
doesn't seem to be implemented for console, but is for ttys, but it
doesn't use bg_check...
-------------- next part --------------
From 2b1c9bd4fb73b514db69023daf031c7e0f6b176c Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <corinna@vinschen.de>
Date: Tue, 26 Jul 2016 19:03:07 +0100
Subject: [PATCH] Don't raise SIGTTIN from poll/select
SIGTTIN should be raised when read() is made on a tty in a backgrounded
process, but not when it's tested with poll()/select().
I guess poll()/select() does need to call bg_check(), in order to detect the
error conditions that notices (that is, if bg_check() returns bg_eof or
bg_error, then fd is ready as an error condition exists) so add an optional
parameter to fhandler_base::bg_select() to indicate that signals aren't
desired.
See https://cygwin.com/ml/cygwin-developers/2016-07/msg00004.html
---
winsup/cygwin/fhandler.h | 4 ++--
winsup/cygwin/fhandler_termios.cc | 43 +++++++++++++++++++++++++++++++++------
winsup/cygwin/select.cc | 4 ++--
3 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 9f9d9bd..3321523 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -427,7 +427,7 @@ public:
{
return dev ().native ();
}
- virtual bg_check_types bg_check (int) {return bg_ok;}
+ virtual bg_check_types bg_check (int, bool = false) {return bg_ok;}
void clear_readahead ()
{
raixput = raixget = ralen = rabuflen = 0;
@@ -1233,7 +1233,7 @@ class fhandler_termios: public fhandler_base
void sigflush ();
int tcgetpgrp ();
int tcsetpgrp (int pid);
- bg_check_types bg_check (int sig);
+ bg_check_types bg_check (int sig, bool dontsignal = false);
virtual DWORD __acquire_output_mutex (const char *fn, int ln, DWORD ms) {return 1;}
virtual void __release_output_mutex (const char *fn, int ln) {}
void echo_erase (int force = 0);
diff --git a/winsup/cygwin/fhandler_termios.cc b/winsup/cygwin/fhandler_termios.cc
index 8bbe301..dc8a19b 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -166,14 +166,43 @@ tty_min::is_orphaned_process_group (int pgid)
return 1;
}
+/*
+ bg_check: check that this process is either in the foreground process group,
+ or if the terminal operation is allowed for processes which are in a
+ background process group.
+
+ If the operation is not permitted by the terminal configuration for processes
+ which are a member of a background process group, return an error or raise a
+ signal as appropriate.
+
+ This handles the following terminal operations:
+
+ write: sig = SIGTTOU
+ read: sig = SIGTTIN
+ change terminal settings: sig = -SIGTTOU
+ (tcsetattr, tcsetpgrp, etc.)
+ peek (poll, select): sig = SIGTTIN, dontsignal = TRUE
+*/
bg_check_types
-fhandler_termios::bg_check (int sig)
+fhandler_termios::bg_check (int sig, bool dontsignal)
{
+ /* Ignore errors:
+ - this process isn't in a process group
+ - tty is invalid
+
+ Everything is ok if:
+ - this process is in the foreground process group, or
+ - this tty is not the controlling tty for this process (???), or
+ - writing, when TOSTOP TTY mode is not set on this tty
+ */
if (!myself->pgid || !tc () || tc ()->getpgid () == myself->pgid ||
myself->ctty != tc ()->ntty ||
((sig == SIGTTOU) && !(tc ()->ti.c_lflag & TOSTOP)))
return bg_ok;
+ /* sig -SIGTTOU is used to indicate a change to terminal settings, where
+ TOSTOP TTY mode isn't considered when determining if we need to send a
+ signal. */
if (sig < 0)
sig = -sig;
@@ -197,19 +226,20 @@ fhandler_termios::bg_check (int sig)
(_main_tls->sigmask & SIGTOMASK (sig));
cygheap->unlock_tls (tl_entry);
- /* If the process is ignoring SIGTT*, then background IO is OK. If
- the process is not ignoring SIGTT*, then the sig is to be sent to
- all processes in the process group (unless the process group of the
- process is orphaned, in which case we return EIO). */
+ /* If the process is blocking or ignoring SIGTT*, then signals are not sent
+ and background IO is allowed */
if (sigs_ignored)
return bg_ok; /* Just allow the IO */
+ /* If the process group of the process is orphaned, return EIO */
else if (tc ()->is_orphaned_process_group (myself->pgid))
{
termios_printf ("process group is orphaned");
set_errno (EIO); /* This is an IO error */
return bg_error;
}
- else
+ /* Otherwise, if signalling is desired, the signal is sent to all processes in
+ the process group */
+ else if (!dontsignal)
{
/* Don't raise a SIGTT* signal if we have already been
interrupted by another signal. */
@@ -222,6 +252,7 @@ fhandler_termios::bg_check (int sig)
}
return bg_signalled;
}
+ return bg_ok;
}
#define set_input_done(x) input_done = input_done || (x)
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 217544e..5789737 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -645,7 +645,7 @@ peek_pipe (select_record *s, bool from_select)
}
}
- if (fh->bg_check (SIGTTIN) <= bg_eof)
+ if (fh->bg_check (SIGTTIN, true) <= bg_eof)
{
gotone = s->read_ready = true;
goto out;
@@ -884,7 +884,7 @@ peek_console (select_record *me, bool)
set_handle_or_return_if_not_open (h, me);
for (;;)
- if (fh->bg_check (SIGTTIN) <= bg_eof)
+ if (fh->bg_check (SIGTTIN, true) <= bg_eof)
return me->read_ready = true;
else if (!PeekConsoleInput (h, &irec, 1, &events_read) || !events_read)
break;
--
2.8.3
More information about the Cygwin-developers
mailing list