pidfd allows a process to keep a race-free handle to a child process, avoiding the problem of PID recycling meaning that a pid_t no longer refers to the process you thought it did. We’d like to be able to use pidfds in GLib for race-free signalling of subprocesses. GLib currently uses posix_spawn() to spawn those subprocesses, though, and it cannot return a pidfd. Could glibc please add a variant of posix_spawn() which returns a pidfd to the caller? I don’t know of the best API for this, but it could either be a new posix_spawn_with_pidfd() function, or be the existing posix_spawn() but somehow return a pidfd via a posix_spawnattr_t.
Relevant discussion in GLib’s issue tracker: https://gitlab.gnome.org/GNOME/glib/-/issues/1866
I think it should be a reasonable interface for Linux. I think the most straightforward interface would be: int posix_spawn_pidfd_ex (int *restrict pidfd, const char *restrict path, const posix_spawn_file_actions_t *restrict file_actions, const posix_spawnattr_t *restrict attrp, char *const argv[restrict_arr], char *const envp[restrict_arr]) Since returning the value on posix_spawnattr_t would require removing the const from it, which is not orthogonal to posix_spawn/posix_spawnp. It will only work on Linux 5.2; but since it is an extension I think it should reasonable.
Another question is if would make sense to also provide the posix_spawnp variant as well, to be orthogonal to POSIX interface.
Another issue is if the new interface would be required to also return the pid_t from he spawned process. Returning it would allow it to work with the default wait interfaces (waitpid, etc.); and it would avoid the possible issue of using this interface with a kernel between 5.2 and 5.4 (since waitid only support for 5.4+). My take is to not support return the pid_t (since posix_spawn{p} already provides it), but now that we don't effoce kernel versions I don't have a good solution for the case kernel does not support waitid (P_PIDFD).
Another corner case, stressed by tst-spawn3.c, is when the process has already reached its maximum number of file descriptors (RLIMIT_NOFILE). Without pidfd, posix_spawn would create the process, while now clone (CLONE_PIDFD) fails. It is expected, but it might be a caveat while using the interface.
(In reply to Adhemerval Zanella from comment #2) > I think it should be a reasonable interface for Linux. I think the most > straightforward interface would be: > > int posix_spawn_pidfd_ex (int *restrict pidfd, const char *restrict path, > const posix_spawn_file_actions_t *restrict > file_actions, > const posix_spawnattr_t *restrict attrp, > char *const argv[restrict_arr], > char *const envp[restrict_arr]) Another option, which would not require adding a new symbol, would to add a new posix_spawnattr_t attribute (POSIX_SPAWN_PIDFD), which would make posix_spawn return the pidfd on the 'pid' argument. It should be safe at least on Linux, where pid_t and int are interchangeable.
After discussion with other maintainers, it seems that it would be better not to reuse the posix_spawn and posix_spawnp interface (with a new flag) and instead add different interfaces. Florian has raised the possible issue with language bindings that might track the argument lifetime and mixing pid_t and file descriptors might generate some problems in such scenarios. And since we already provide wrappers over the Linux pidfd syscall, Florian also suggested using the same name convention. The proposed interfaces are: extern int pidfd_spawn (int *__restrict __pid, const char *__restrict __path, const posix_spawn_file_actions_t *__restrict __facts, const posix_spawnattr_t *__restrict __attrp, char *const __argv[__restrict_arr], char *const __envp[__restrict_arr]) __nonnull ((2, 5)); extern int pidfd_spawnp (int *__restrict __pid, const char *__restrict __path, const posix_spawn_file_actions_t *__restrict __facts, const posix_spawnattr_t *__restrict __attrp, char *const __argv[__restrict_arr], char *const __envp[__restrict_arr]) __nonnull ((2, 5)); I decided to reuse the posix_spawn attribute and file actions to avoid either to rehash posix_spawn API or add a new one. This new API will be provided by spawn.h.
Proposed patch sent upstream - https://patchwork.sourceware.org/project/glibc/list/?series=19063
I’ve looked over the latest patches in https://patchwork.sourceware.org/project/glibc/list/?series=19143, and it all looks good to me! I focused on looking at whether it would cover GLib’s needs, and whether the new API made sense and felt natural and complete. I have not done a line-by-line review or looked in detail at the /proc parsing code. So, from the point of view of GLib, the proposed APIs look great. Thanks a lot for working on them.
The new API looks great also from systemd's point of view, thanks for your work, looking forward to using these!
Implemented on 2.39 (0d6f9f626521678f330f8bfee89e1cdb7e2b1062) by pidfd_spawn/pidfd_spawnp.
Fantastic! Thanks for your work on this.