This commit causes issues with job control on hppa: commit 342cc934a3bf74ac618e2318d738f22ac93257ba (HEAD) Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> AuthorDate: Mon Jun 14 14:41:31 2021 -0300 Commit: Adhemerval Zanella <adhemerval.zanella@linaro.org> CommitDate: Tue Jan 25 14:07:53 2022 -0300 posix: Add terminal control setting support for posix_spawn Currently there is no proper way to set the controlling terminal through posix_spawn in race free manner [1]. This forces shell implementations to keep using fork+exec when launching background process groups, even when using posix_spawn yields better performance. This patch adds a new GNU extension so the creating process can configure the created process terminal group. This is done with a new flag, POSIX_SPAWN_TCSETPGROUP, along with two new attribute functions: posix_spawnattr_tcsetpgrp_np, and posix_spawnattr_tcgetpgrp_np. The function sets a new attribute, spawn-tcgroupfd, that references to the controlling terminal. The controlling terminal is set after the spawn-pgroup attribute, and uses the spawn-tcgroupfd along with current creating process group (so it is composable with POSIX_SPAWN_SETPGROUP). To create a process and set the controlling terminal, one can use the following sequence: posix_spawnattr_t attr; posix_spawnattr_init (&attr); posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP); posix_spawnattr_tcsetpgrp_np (&attr, tcfd); If the idea is also to create a new process groups: posix_spawnattr_t attr; posix_spawnattr_init (&attr); posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP | POSIX_SPAWN_SETPGROUP); posix_spawnattr_tcsetpgrp_np (&attr, tcfd); posix_spawnattr_setpgroup (&attr, 0); The controlling terminal file descriptor is ignored if the new flag is not set. This interface is slight different than the one provided by QNX [2], which only provides the POSIX_SPAWN_TCSETPGROUP flag. The QNX documentation does not specify how the controlling terminal is obtained nor how it iteracts with POSIX_SPAWN_SETPGROUP. Since a glibc implementation is library based, it is more straightforward and avoid requires additional file descriptor operations to request the caller to setup the controlling terminal file descriptor (and it also allows a bit less error handling by posix_spawn). Checked on x86_64-linux-gnu and i686-linux-gnu. [1] https://github.com/ksh93/ksh/issues/79 [2] https://www.qnx.com/developers/docs/7.0.0/index.html#com.qnx.doc.neutrino.lib_ref/topic/p/posix_spawn.html Reviewed-by: Carlos O'Donell <carlos@redhat.com> Tested-by: Carlos O'Donell <carlos@redhat.com> When I do a glibc build and check, I use a script as follows: nohup ../build > build.log 2>&1 & I usually monitor progress with less: less build.log At some point in the check, less stops and goes into the background. I think this occurs when tst-spawn6 runs. It appears that the test affects processes beyond those spawned in test. This occurs with all kernel versions that I have tried. With 5.16.4, I also see general problems with job control after running the glibc testsuite. Here is output from ssh login: Using username "dave". Authenticating with public key "imported-openssh-key" Linux mx3210 5.16.4+ #1 SMP Sat Jan 29 18:56:52 UTC 2022 parisc64 The programs included with the Debian GNU/Linux system are free software; the exact distribution terms for each program are described in the individual files in /usr/share/doc/*/copyright. Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent permitted by applicable law. You have no mail. Last login: Sat Jan 29 20:27:13 2022 from 192.168.2.49 -bash: cannot set terminal process group (-1): Bad file descriptor -bash: no job control in this shell This doesn't seem to occur if glibc testsuite hasn't been run.
Since hppa is big endian, this could be an endian bug clobbering the terminal process group.
I suppose this is the problematic part of the change: /* Set the controlling terminal. */ if ((attr->__flags & POSIX_SPAWN_TCSETPGROUP) != 0) { /* Check if it is possible to avoid an extra syscall. */ pid_t pgrp = (attr->__flags & POSIX_SPAWN_SETPGROUP) != 0 && attr->__pgrp != 0 ? attr->__pgrp : __getpgid (0); if (__tcsetpgrp (attr->__ctty_fd, pgrp) != 0) goto fail; }
The function tcsetpgrp() makes the process group with process group ID pgrp the foreground process group on the terminal associated to fd, which must be the controlling terminal of the calling process, and still be associated with its session. Moreover, pgrp must be a (nonempty) process group belonging to the same session as the calling process. If tcsetpgrp() is called by a member of a background process group in its session, and the calling process is not blocking or ignoring SIGT- TOU, a SIGTTOU signal is sent to all members of this background process group.
Dave, Thanks for the bug report. I can confirm that posix/tst-spawn6 breaks out of the support/* isolation and in doing so impacts the shell / process group / session on the terminal. e.g. [2]+ Stopped less ~/build/glibc-review/check.log (In reply to John David Anglin from comment #2) > I suppose this is the problematic part of the change: > > /* Set the controlling terminal. */ > if ((attr->__flags & POSIX_SPAWN_TCSETPGROUP) != 0) > { > /* Check if it is possible to avoid an extra syscall. */ > pid_t pgrp = (attr->__flags & POSIX_SPAWN_SETPGROUP) != 0 > && attr->__pgrp != 0 > ? attr->__pgrp : __getpgid (0); > if (__tcsetpgrp (attr->__ctty_fd, pgrp) != 0) > goto fail; > } This is not problematic per-se, since this is what you want to do for shell control, but if the test isolation code doesn't stop this affecting your current shell then that's a problem. The posix/tst-spawn6 test is the *only* test in which we exercise process group control testing, and so it may break the testing isolation we expect. (In reply to John David Anglin from comment #3) > The function tcsetpgrp() makes the process group with process group ID > pgrp the foreground process group on the terminal associated to fd, > which must be the controlling terminal of the calling process, and > still be associated with its session. Moreover, pgrp must be a > (nonempty) process group belonging to the same session as the calling > process. > If tcsetpgrp() is called by a member of a background process group in > its session, and the calling process is not blocking or ignoring SIGT- > TOU, a SIGTTOU signal is sent to all members of this background process > group. Yes, for the current /dev/tty (in the test), we make the posix_spawn'd process the foreground process group for the session to verify the test works correctly. I can't easily containerize the test because we don't setup an alternative TTY/session/pgrp in the container e.g. tests-container += tst-spawn6. make[2]: Leaving directory '/home/carlos/src/glibc-review/posix' FAIL: posix/tst-spawn6 original exit status 1 error: tst-spawn6.c:138: open64 ("/dev/tty", 0x0, 0600): No such file or directory error: 1 test failures make[1]: Leaving directory '/home/carlos/src/glibc-review'
The solution here is that the test needs it's own session with a new process group, and new terminal. So posix/tst-spawn6 needs some additional work to avoid affecting build systems.
(In reply to Carlos O'Donell from comment #5) > The solution here is that the test needs it's own session with a new process > group, and new terminal. So posix/tst-spawn6 needs some additional work to > avoid affecting build systems. And we have a fix posted for this: https://sourceware.org/pipermail/libc-alpha/2022-February/136033.html
Fixed on 2.36.
As per last comment.
*** Bug 28836 has been marked as a duplicate of this bug. ***