[bubblewrap] Propagate SIGTERM and SIGINT to child

[earlchew]

I'm proposing this commit to address #369.

Instead of the default termination when receiving SIGINT or SIGTERM, this change propagates SIGINT and SIGTERM from the parent to the child.

[rh-atomic-bot]

Can one of the admins verify this patch?
I understand the following commands:

• bot, add author to whitelist
• bot, test pull request
• bot, test pull request once

[ntrrgc]

The proposed patch works and it's necessary to be able to use ^C in flatpak run. (Without being thrown to the shell when it should be handled by the sandboxed process instead, that is.)

[infinisil]

I can confirm that this patch works well! In my case I'm using bubblewrap with cabal repl, which normally supports using Ctrl-C to clear the current line (as do most other repls), but within bubblewrap it would exit the shell completely. This patch makes it work as intended.

[smcv]

I think it should certainly be possible to make bubblewrap catch and forward signals, but I'm less sure that it should be catching and forwarding them by default - there's a risk that someone is relying on its current behaviour.

On #402, @wansing also wanted to catch and forward SIGHUP. It would probably make sense to do the same for SIGUSR1, SIGUSR2 and SIGQUIT.

Also on #402, @valentindavid thought the CLI use-case would be better handled by putting bubblewrap's child in a new process group, and making that process group the foreground process group for the terminal, so that Ctrl+C and Ctrl+\ deliver SIGINT and SIGQUIT to the child instead of to bwrap.

[valentindavid]

I do not think this is the proper way to solve #369. We need to properly set the foreground process group.

[sqwishy]

I do not think this is the proper way to solve #369. We need to properly set the foreground process group.

A couple questions.

1. If bwrap is no longer the foreground process group, then if it tries to write to the terminal it will be receive a signal (SIGTTOU?) and be suspended or something, is that right? And might that ever happen? I tried using --info-fd 2 but I think that happens before tcsetpgrp() is/would be called so it doesn't really matter. I'm curious if this could happen in another way.

2. Does this help at all when running bwrap under supervision like as a systemd service?

   Typically stopping a service in systemd will send a signal like SIGTERM to the supervised process (although this can be configured and maybe using GuessMainPID= or PIDFile= are better options). So making a foreground new process group for the terminal might not do anything to solve this.

   Not that it shouldn't be done anyway! Maybe it's still a cool and useful thing on its own. But forwarding signals to the child might also be required?

   Also, for what it's worth, I think it might be important that the user be able to specify the signals to forward. It would be a shame to implement this but have it be useless for users because it omitted signals (like SIGUSR1 as mentioned by someone else earlier).

[bendlas]

I'm using this to run a valheim server using NixOS' steam-run. Can confirm that this patch together with KillSignal = "SIGINT"; makes the world save correctly on stop.

[luke-jr]

How is this not fixed after 2 years? Using this as part of my compile sandbox, but now Ctrl-C is leaving corrupt object files -.-

[alxchk]

Looks like without this patch bwrap is useless for interactive cli tools, which handle SIGINT

[bendlas]

I think it should certainly be possible to make bubblewrap catch and forward signals, but I'm less sure that it should be catching and forwarding them by default - there's a risk that someone is relying on its current behaviour.

@smcv if somebody made config options for selectively forwarding any signals, would you be willing to help push that forward?

[smcv]

if somebody made config options for selectively forwarding any signals, would you be willing to help push that forward?

I'd review a pull request, if that's what you mean.

[earlchew]

@smcv wrote:

I think it should certainly be possible to make bubblewrap catch and forward signals, but I'm less sure that it should be catching and forwarding them by default - there's a risk that someone is relying on its current behaviour.

That's a fair call given the established user base. Ideas to accommodate this might include:

• A --interactive option to support working with a controlling terminal
• A --signal option to forward additional or specific signals

An implementation could support just one, or both, these strategies.

Thinking more broadly, most generally the lifecycle of a process can be controlled by kill(2) or killpg(2). Placing the child process in a new process group allows signals sent by killpg(2) to be restricted to the new group, and exclude the parent. This is of particular use when the new process group is configured as the foreground process of a controlling terminal, but processes can run without a controlling terminal. Additionally termination signals need not just arise from the controlling terminal, but could be generated by the launching process (perhaps a shell or shell script) or even some process with the right permissions.

Ideally, I think it is least surprising if the following hold even when there is no controlling terminal:

• The parent process shares the fate of the child process.
• The child process shares the fate of the parent process.

The consequence of the above is:

• If a child process is stopped, the parent process shall stop.
• If a parent process is stopped, the child process shall stop.
• If a child process terminates by signal, the parent process shall terminate by the same signal.
• If a parent process receives a signal, the child process shall receive the same signal.
• If a child exits with a status code, the parent process shall exit with the same status code.

If both the parent and child reside in the same process group, then signals sent to the shared process group (whether sent by killpg(2) or sent by via the controlling terminal) are delivered to both parent and child. Because of the shared process group, job control signals are delivered to both parent and child without additional configuration. The parent must block termination signals, propagate delivery to the child, and only mimic the behaviour of the child.

If the parent and child reside in different process groups, then signals sent to the process group of the parent will not be delivered to the child. This means that job control signals sent via the controlling terminal must be directed to the child otherwise a SIGSTOP will prevent the parent from suspending the child. The requirement for shared fate means the parent must mimic suspension of the child (suspended also perhaps due to SIGTTOU), and later propagate continuation to the child. This means that the parent must use tcsetpgrp(2) to interact with the controlling terminal when available to achieve the expected behaviour.

Even when the parent and child reside in different process groups, the parent must block termination signals, propagate delivery to the child because these signals might be sent by other processes independently of the controlling terminal, and mimic the behaviour of the child to meet the expectations of the launching process.

Given the above, the only reasons I could think of for placing the child in a new process group are:

• To convey to the child that the launching process intended to start a new "Process Group Leader" and that the child should behave with the corresponding responsibilities.
• To avoid a surprising answer to getpgid(2) when CLONE_NEWPID is used, particularly if the child is trying to restore its original process group (see https://stackoverflow.com/a/50592189 or https://pubs.opengroup.org/onlinepubs/009604599/functions/setpgid.html), or applications that make assumptions about positive non-zero pgids (perhaps for example https://git.savannah.gnu.org/gitweb/?p=bash.git;a=blob;f=jobs.c;h=6b986ed76ace7e249198d61c04aff6da26f02823;hb=74091dd4e8086db518b30df7f222691524469998#l2268)

[smcv]

Superseded by #586, which seems more likely to reach a mergeable state.