http://austingroupbugs.net/view.php?id=411 proposes adding more atomic FD_CLOEXEC into POSIX interfaces, in part based on what glibc has done. But it picked up on one thing that glibc has not yet changed: if you have a multi-threaded app that has been converted to properly use O_CLOEXEC on ALL file opens, and then want to hand-choose which fds are then purposefully inherited into a child process, your normal course of action is to use fcntl(F_SETFD) in between fork() and exec*() - but there is no counterpart to this when using posix_spawn() instead of fork/exec. Unfortunately, using posix_spawn_file_actions_adddup2(&act, fd, fd) is currently a no-op in glibc - if fd already has FD_CLOEXEC set, the child process will not inherit fd. A smarter course of action is to have the dup2 action in the file_actions list result in the proper fcntl() in the child process, as a way of intentionally marking an fd as inherited into that particular child while still remaining FD_CLOEXEC in the parent. Here's what the POSIX wording has to say: "Also, I don't know of any platform where posix_spawn_file_actions_adddup2( ) can clear FD_CLOEXEC, but that seems like a useful change to make while tightening the specification on how to properly use FD_CLOEXEC, since the only standardized alternative for explicitly handing a file descriptor to the child process while leaving it FD_CLOEXEC in the parent process involves more complexity and risks EMFILE failure. ... At line 46976 [XSH posix_spawn_file_actions_adddup2], add a sentence: If fildes and newfildes are equal, then the action shall ensure that the FD_CLOEXEC flag of fildes is cleared (even though dup2( ) would leave it unchanged). After line 46999 [XSH posix_spawn_file_actions_adddup2], add the following: Although dup2( ) is required to do nothing when fildes and newfildes are equal and fildes is an open descriptor, the use of posix_spawn_file_actions_adddup2( ) is required to clear the FD_CLOEXEC flag of fildes. This is because there is no counterpart of posix_spawn_file_actions_fcntl( ) that could be used for clearing the flag; it would also be possible to achieve this effect by using two calls to posix_spawn_file_actions_adddup2( ) and a temporary fildes value known to not conflict with any other file descriptors, coupled with a posix_spawn_file_actions_close( ) to avoid leaking the temporary, but this approach is complex, and risks EMFILE or ENFILE failure that can be avoided with the in-place removal of FD_CLOEXEC. There is no need for posix_spawn_file_actions_adddup3( ), since it makes no sense to create a file descriptor with FD_CLOEXEC set before spawning the child process, where that file descriptor would immediately be closed again.
The attachment to POSIX bug 411 at http://austingroupbugs.net/file_download.php?file_id=36&type=bug has a slightly updated wording proposal: At page 1433 line 46976 [XSH posix_spawn_file_actions_adddup2( ) DESCRIPTION], add a sentence: If fildes and newfildes are equal, then the action shall ensure that newfildes is inherited by the new process with FD_CLOEXEC clear, even if the FD_CLOEXEC flag of fildes is set at the time the new process is spawned, and even though dup2( ) would not make such a change.
I think the rationale to have a posix_spawn operation to reset O_CLOEXEC is reasonable, but I am not very found of having dup2 and posix_spawn_file_actions_adddup2 with subtle different semantics: this usually lead to wrong assumptions, since posix_spawn file actions are in fact modeled based on POSIX counterparts. I am not sure which would be the best extension to add though (maybe posix_spawn_file_actions_fcntl?).
(In reply to Adhemerval Zanella from comment #2) > I think the rationale to have a posix_spawn operation to reset O_CLOEXEC is > reasonable, but I am not very found of having dup2 and > posix_spawn_file_actions_adddup2 with subtle different semantics: this > usually lead to wrong assumptions, since posix_spawn file actions are in > fact modeled based on POSIX counterparts. > > I am not sure which would be the best extension to add though (maybe > posix_spawn_file_actions_fcntl?). Adding posix_spawn_file_actions_addfcntl seems like a lot of complexity. In particular, the fact that you have to read-modify-write in order to properly use fcntl() makes it rather verbose to have to add multiple fcntl actions to file_actions, compared to having a single call just give the semantics you want. Another drawback of adding an _addfcntl is deciding what things it supports, as well as documenting the vast majority of things it specifically does not support (updating file locking, for example, seems like a huge growth in scope and unlikely to ever be natively part of a spawn() syscall, but having a spawn syscall that can clear FD_CLOEXEC on fds specifically requested for use in the child does not seem difficult). A posix_spawn_file_actions_adddup3() doesn't make much sense either - the normal dup3() is specifically documented to require failure on dup3(fd, fd, flags) (unlike dup2(fd, fd) which must be a no-op). You could go with an even more simplistic approach, and add posix_spawn_file_actions_addnocloexec(&act, fd) (instead of _adddup2(&act, fd, fd)), where the different function name has no direct bare counterpart, but makes it immediately obvious that its goal is to remove FD_CLOEXEC flags on fds that must remain cloexec in the parent, but must be inherited by the child. Of course, if glibc actually implements something different than the current POSIX proposal, it's not too late to use that as feedback to update the POSIX proposal to match existing practice (after all, the proposal is still just that, and hasn't actually been incorporated into a released version of POSIX yet). I'm just pointing out that when this proposal was first vetted by the Austin Group (several years ago now), no one had any better ideas for an easy way to clear FD_CLOEXEC, besides making _adddup2() have a documented slightly different behavior from normal dup2().
(In reply to Eric Blake from comment #3) > (In reply to Adhemerval Zanella from comment #2) > > I think the rationale to have a posix_spawn operation to reset O_CLOEXEC is > > reasonable, but I am not very found of having dup2 and > > posix_spawn_file_actions_adddup2 with subtle different semantics: this > > usually lead to wrong assumptions, since posix_spawn file actions are in > > fact modeled based on POSIX counterparts. > > > > I am not sure which would be the best extension to add though (maybe > > posix_spawn_file_actions_fcntl?). > > Adding posix_spawn_file_actions_addfcntl seems like a lot of complexity. In > particular, the fact that you have to read-modify-write in order to properly > use fcntl() makes it rather verbose to have to add multiple fcntl actions to > file_actions, compared to having a single call just give the semantics you > want. Another drawback of adding an _addfcntl is deciding what things it > supports, as well as documenting the vast majority of things it specifically > does not support (updating file locking, for example, seems like a huge > growth in scope and unlikely to ever be natively part of a spawn() syscall, > but having a spawn syscall that can clear FD_CLOEXEC on fds specifically > requested for use in the child does not seem difficult). I agree addfcntl would add a lot of complexity and we would need to add extensive documentation it is only intended to provide a subset of default fcntl operations (which I also think it is not a good idea). > > A posix_spawn_file_actions_adddup3() doesn't make much sense either - the > normal dup3() is specifically documented to require failure on dup3(fd, fd, > flags) (unlike dup2(fd, fd) which must be a no-op). Agreed. > > You could go with an even more simplistic approach, and add > posix_spawn_file_actions_addnocloexec(&act, fd) (instead of _adddup2(&act, > fd, fd)), where the different function name has no direct bare counterpart, > but makes it immediately obvious that its goal is to remove FD_CLOEXEC flags > on fds that must remain cloexec in the parent, but must be inherited by the > child. > > Of course, if glibc actually implements something different than the current > POSIX proposal, it's not too late to use that as feedback to update the > POSIX proposal to match existing practice (after all, the proposal is still > just that, and hasn't actually been incorporated into a released version of > POSIX yet). I'm just pointing out that when this proposal was first vetted > by the Austin Group (several years ago now), no one had any better ideas for > an easy way to clear FD_CLOEXEC, besides making _adddup2() have a documented > slightly different behavior from normal dup2(). It looks like some libcs (musl for instance) are already pushing this proposal as expected behavior and I would like to avoid either adding a non-portable extension or deviating from POSIX. I will work on adding this support on GLIBC, thanks to bring this up.
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, master has been updated via 805334b26c7e6e83557234f2008497c72176a6cd (commit) from 03992356e6fedc5a5e9d32df96c1a2c79ea28a8f (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=805334b26c7e6e83557234f2008497c72176a6cd commit 805334b26c7e6e83557234f2008497c72176a6cd Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> Date: Wed Sep 19 12:14:34 2018 -0700 posix: Clear close-on-exec for posix_spawn adddup2 (BZ#23640) Austin Group issue #411 [1] proposes that posix_spawn file action posix_spawn_file_actions_adddup2 resets the close-on-exec when source and destination refer to same file descriptor. It solves the issue on multi-thread applications which uses close-on-exec as default, and want to hand-chose specifically file descriptor to purposefully inherited into a child process. Current approach to achieve this scenario is to use two adddup2 file actions and a temporary file description which do not conflict with any other, coupled with a close file action to avoid leaking the temporary file descriptor. This approach, besides being complex, may fail with EMFILE/ENFILE file descriptor exaustion. This can be more easily accomplished with an in-place removal of FD_CLOEXEC. Although the resulting adddup2 semantic is slight different than dup2 (equal file descriptors should be handled as no-op), the proposed possible solution are either more complex (fcntl action which a limited set of operations) or results in unrequired operations (dup3 which also returns EINVAL for same file descriptor). Checked on aarch64-linux-gnu. [BZ #23640] * posix/tst-spawn.c (do_prepare, handle_restart, do_test): Add posix_spawn_file_actions_adddup2 test to check O_CLOCEXEC reset. * sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Add close-on-exec reset for adddup2 file action. * sysdeps/posix/spawni.c (__spawni_child): Likewise. [1] http://austingroupbugs.net/view.php?id=411 ----------------------------------------------------------------------- Summary of changes: ChangeLog | 9 +++++++ posix/tst-spawn.c | 44 +++++++++++++++++++++++++++++++------- sysdeps/posix/spawni.c | 18 +++++++++++++-- sysdeps/unix/sysv/linux/spawni.c | 18 +++++++++++++-- 4 files changed, 75 insertions(+), 14 deletions(-)
Fixed on 2.29 (commit 805334b26c).