* Re: [glibc] posix: Add terminal control setting support for posix_spawn [not found] <20220125180124.B88AC3857C70@sourceware.org> @ 2022-01-27 10:49 ` Florian Weimer 2022-01-27 11:38 ` Adhemerval Zanella 0 siblings, 1 reply; 5+ messages in thread From: Florian Weimer @ 2022-01-27 10:49 UTC (permalink / raw) To: libc-alpha * Adhemerval Zanella via Glibc-cvs: > commit 342cc934a3bf74ac618e2318d738f22ac93257ba > Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> > Date: Mon Jun 14 14:41:31 2021 -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); Sorry, I did not look at this in detail so far. A file descriptor outside the file actions API makes me quite nervous. Advanced Programming in the UNIX Environment says (like POSIX), “If the process has a controlling terminal, the process can call tcsetpgrp to set the foreground process group ID […]”. This suggests to me that a file action may be required to establish a controlling terminal. Callers of posix_spawn might restrict that to the spawned process. As far as I know, Linux follows SVR4, and opening the terminal device under /dev/pts/ in a session leader that has no controlling terminal establishes one. This translate to a posix_spawn file action. So the action established by posix_spawnattr_tcsetpgrp_np must come after the file actions. But the descriptor used by posix_spawnattr_tcsetpgrp_np must still be open. So it would have to come before a closefrom file action (or a regular close). I think this means that the action established by posix_spawnattr_tcsetpgrp_np needs to be sequenced among the file actions, and needs to be a file action itself. Thanks, Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [glibc] posix: Add terminal control setting support for posix_spawn 2022-01-27 10:49 ` [glibc] posix: Add terminal control setting support for posix_spawn Florian Weimer @ 2022-01-27 11:38 ` Adhemerval Zanella 2022-01-27 11:57 ` Florian Weimer 0 siblings, 1 reply; 5+ messages in thread From: Adhemerval Zanella @ 2022-01-27 11:38 UTC (permalink / raw) To: Florian Weimer, libc-alpha On 27/01/2022 07:49, Florian Weimer via Libc-alpha wrote: > * Adhemerval Zanella via Glibc-cvs: > >> commit 342cc934a3bf74ac618e2318d738f22ac93257ba >> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> Date: Mon Jun 14 14:41:31 2021 -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); > > Sorry, I did not look at this in detail so far. A file descriptor > outside the file actions API makes me quite nervous. > > Advanced Programming in the UNIX Environment says (like POSIX), “If the > process has a controlling terminal, the process can call tcsetpgrp to > set the foreground process group ID […]”. This suggests to me that a > file action may be required to establish a controlling terminal. > Callers of posix_spawn might restrict that to the spawned process. As > far as I know, Linux follows SVR4, and opening the terminal device under > /dev/pts/ in a session leader that has no controlling terminal > establishes one. This translate to a posix_spawn file action. So the > action established by posix_spawnattr_tcsetpgrp_np must come after the > file actions. > > But the descriptor used by posix_spawnattr_tcsetpgrp_np must still be > open. So it would have to come before a closefrom file action (or a > regular close). > > I think this means that the action established by > posix_spawnattr_tcsetpgrp_np needs to be sequenced among the file > actions, and needs to be a file action itself. > > Thanks, > Florian > I did not considered the closefrom in conjunction with closefrom file action and making a file action does make sense indeed. Maybe a better approach would be to remove posix_spawnattr_tcsetpgrp_np and posix_spawnattr_tcsetpgrp_np and add: int posix_spawn_file_actions_tcsetgrp_np (posix_spawn_file_actions_t *, int fd); Similar to posix_spawn_file_actions_addfchdir_np. So to create a process and set the controlling terminal, one can use the following sequence: int tcfd = open (_PATH_TTY, O_RDONLY, 0600); posix_spawnattr_t attr; posix_spawnattr_init (&attr); posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP); posix_spawn_file_actions_t actions; posix_spawn_file_actions_init (&actions); posix_spawn_file_actions_tcsetgrp_np (&actions, tcfd); So users would be able to add a posix_spawn_file_actions_addclose or posix_spawn_file_actions_addclosefrom_np. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [glibc] posix: Add terminal control setting support for posix_spawn 2022-01-27 11:38 ` Adhemerval Zanella @ 2022-01-27 11:57 ` Florian Weimer 2022-01-27 12:02 ` Adhemerval Zanella 0 siblings, 1 reply; 5+ messages in thread From: Florian Weimer @ 2022-01-27 11:57 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha * Adhemerval Zanella: > I did not considered the closefrom in conjunction with closefrom file > action and making a file action does make sense indeed. Maybe a better > approach would be to remove posix_spawnattr_tcsetpgrp_np and > posix_spawnattr_tcsetpgrp_np and add: > > int posix_spawn_file_actions_tcsetgrp_np (posix_spawn_file_actions_t *, > int fd); > > Similar to posix_spawn_file_actions_addfchdir_np. > > So to create a process and set the controlling terminal, one can use the > following sequence: > > int tcfd = open (_PATH_TTY, O_RDONLY, 0600); > > posix_spawnattr_t attr; > posix_spawnattr_init (&attr); > posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP); Why is POSIX_SPAWN_TCSETPGROUP needed? > posix_spawn_file_actions_t actions; > posix_spawn_file_actions_init (&actions); > posix_spawn_file_actions_tcsetgrp_np (&actions, tcfd); > > So users would be able to add a posix_spawn_file_actions_addclose or > posix_spawn_file_actions_addclosefrom_np. This would address my concern, yes. Thanks, Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [glibc] posix: Add terminal control setting support for posix_spawn 2022-01-27 11:57 ` Florian Weimer @ 2022-01-27 12:02 ` Adhemerval Zanella 2022-01-27 12:15 ` Florian Weimer 0 siblings, 1 reply; 5+ messages in thread From: Adhemerval Zanella @ 2022-01-27 12:02 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha On 27/01/2022 08:57, Florian Weimer wrote: > * Adhemerval Zanella: > >> I did not considered the closefrom in conjunction with closefrom file >> action and making a file action does make sense indeed. Maybe a better >> approach would be to remove posix_spawnattr_tcsetpgrp_np and >> posix_spawnattr_tcsetpgrp_np and add: >> >> int posix_spawn_file_actions_tcsetgrp_np (posix_spawn_file_actions_t *, >> int fd); >> >> Similar to posix_spawn_file_actions_addfchdir_np. >> >> So to create a process and set the controlling terminal, one can use the >> following sequence: >> >> int tcfd = open (_PATH_TTY, O_RDONLY, 0600); >> >> posix_spawnattr_t attr; >> posix_spawnattr_init (&attr); >> posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP); > > Why is POSIX_SPAWN_TCSETPGROUP needed? The idea would so caller can check if the action would take, but it does not make sense with a file action. > >> posix_spawn_file_actions_t actions; >> posix_spawn_file_actions_init (&actions); >> posix_spawn_file_actions_tcsetgrp_np (&actions, tcfd); In fact I think it should be posix_spawn_file_actions_addtcsetgrp_np to follow other file action naming convention. >> >> So users would be able to add a posix_spawn_file_actions_addclose or >> posix_spawn_file_actions_addclosefrom_np. > > This would address my concern, yes. > > Thanks, > Florian > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [glibc] posix: Add terminal control setting support for posix_spawn 2022-01-27 12:02 ` Adhemerval Zanella @ 2022-01-27 12:15 ` Florian Weimer 0 siblings, 0 replies; 5+ messages in thread From: Florian Weimer @ 2022-01-27 12:15 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha * Adhemerval Zanella: >>> posix_spawn_file_actions_t actions; >>> posix_spawn_file_actions_init (&actions); >>> posix_spawn_file_actions_tcsetgrp_np (&actions, tcfd); > > In fact I think it should be posix_spawn_file_actions_addtcsetgrp_np > to follow other file action naming convention. Yes, that name is better. Thanks, Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-27 12:15 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20220125180124.B88AC3857C70@sourceware.org> 2022-01-27 10:49 ` [glibc] posix: Add terminal control setting support for posix_spawn Florian Weimer 2022-01-27 11:38 ` Adhemerval Zanella 2022-01-27 11:57 ` Florian Weimer 2022-01-27 12:02 ` Adhemerval Zanella 2022-01-27 12:15 ` Florian Weimer
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 read-only IMAP folder(s) and NNTP newsgroup(s).