Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command-pid member to --json-status-fd #576

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

WGH-
Copy link

@WGH- WGH- commented Apr 17, 2023

In cases where bwrap runs its own additional PID 1 process (e.g. --unshare-pid), child-pid is the PID of that process, not the PID of the user-specified COMMAND.

This is inconvenient as the user might want to send e.g. SIGTERM to the command to give it a chance to exit gracefully.

Example:

$ ./bwrap --json-status-fd 2 -- /bin/true
{ "child-pid": 2779366, "mnt-namespace": 4026538517 }
{ "command-pid": 2779366 }

$ ./bwrap --json-status-fd 2 --unshare-pid -- /bin/true
{ "child-pid": 2779372, "mnt-namespace": 4026538517, "pid-namespace": 4026538518 }
{ "command-pid": 2779373 }

Closes #553

@WGH-
Copy link
Author

WGH- commented Apr 17, 2023

Note that unlike child-pid this is currently impossible to use while being immune to PID reuse.

For child-pid you can do something like this:

pid = receive_pid_from_json_status_fd()
pidfd = pidfd_open(fd)
# if block_fd is not closed, then child-pid certainly was alive at the time of pidfd_open() call
if closed(block_fd):
    close(pidfd)
    return None
return pidfd

Unless I'm missing some simple solution, we'll have to introduce another block_fd-like fd that would be leaked to the COMMAND process (and even then COMMAND may close it early, making the code inadvertently fail).

bwrap.xml Outdated Show resolved Hide resolved
@WGH-
Copy link
Author

WGH- commented Jun 10, 2024

Heh. I'll rebase and fix conflicts in a day or so.

In cases where bwrap runs its own additional PID 1 process (e.g.
--unshare-pid), child-pid is the PID of that process, not the
PID of the user-specified COMMAND.

This is inconvenient as the user might want to send e.g. SIGTERM to the
command to give it a chance to exit gracefully.

Closes containers#553

Signed-off-by: WGH <wgh@torlan.ru>
@WGH-
Copy link
Author

WGH- commented Jun 11, 2024

Rebased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: --json-status-fd could provide pid of command?
2 participants