poll()/select() on a tty fd in a backgrounded process raises SIGTTIN

[Jon Turney]

Consider the following:

$ cat sigttin-test.c
#include <unistd.h>
#include <poll.h>

// gcc sigttin-test.c -o sigttin-test
// ./sigttin-test &

int main()
{
// poll stdin
struct pollfd fds;
fds.fd = STDIN_FILENO;
fds.events = POLLIN;
return poll (&fds, 1, 0);
}

$ gcc sigttin-test.c -o sigttin-test

on cygwin:

$ ./sigttin-test &
[1] 6120

[1]+ Stopped ./sigttin-test

$ jobs -l
[1]+ 6120 Stopped (tty input) ./sigttin-test


on linux:

$ ./sigttin-test &
[1] 2779
$

My reading seems to indicate that SIGTTIN should be raised when read()
is made on a tty in a backgrounded process, not when it's polled, but
looking at the Cygwin source code, it's been this way since the earliest
versions in git...

This test case is reduced from a problem seen with gdb, since commit
0b333c5e, where gdb stops with SIGTTIN when the inferior is started.

[Corinna Vinschen]

Post by Jon Turney
$ cat sigttin-test.c
#include <unistd.h>
#include <poll.h>
// gcc sigttin-test.c -o sigttin-test
// ./sigttin-test &
int main()
{
// poll stdin
struct pollfd fds;
fds.fd = STDIN_FILENO;
fds.events = POLLIN;
return poll (&fds, 1, 0);
}
$ gcc sigttin-test.c -o sigttin-test
$ ./sigttin-test &
[1] 6120
[1]+ Stopped ./sigttin-test
$ jobs -l
[1]+ 6120 Stopped (tty input) ./sigttin-test
$ ./sigttin-test &
[1] 2779
$
My reading seems to indicate that SIGTTIN should be raised when read() is
made on a tty in a backgrounded process, not when it's polled, but looking
at the Cygwin source code, it's been this way since the earliest versions in
git...
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?


Thanks,
Corinna

[Jon Turney]

Post by Corinna Vinschen

Post by Jon Turney
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...

[Corinna Vinschen]

Post by Corinna Vinschen

Post by Jon Turney
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...

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


Corinna

[Jon Turney]

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Jon Turney
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.

Post by Corinna Vinschen
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...

[Corinna Vinschen]

Post by Jon Turney

Post by Jon Turney
This test case is reduced from a problem seen with gdb, since commit
0b333c5e, where gdb stops with SIGTTIN when the inferior is started.

[...]

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.
[...]
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.

Jon, please don't hesitate to create a followup patch to clear things up.

Post by Jon Turney
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...

Same here, if you have an improvement of this code, feel free.

I wonder why we don't support a fully functional TIOCSWINSZ on consoles
anyway. It shouldn't be that hard, should it? Basically a call to
SetConsoleWindowInfo...

Thanks for improving the comments. Please apply.


Thanks,
Corinna