bubblewrap icon indicating copy to clipboard operation
bubblewrap copied to clipboard

ctrl+c sends SIGINT to bubblwrap instead of child process

Open valentindavid opened this issue 5 years ago • 7 comments

Here is a dumb python program where the issue happens:

import signal
import asyncio

loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)

def interrupt():
    s = input("test")
    print(s)
    loop.stop()

loop.add_signal_handler(signal.SIGINT, interrupt)

loop.run_forever()

loop.remove_signal_handler(signal.SIGINT)

loop.close()

On host, running this program, then pressing ctrl+c, a prompt shows up, then I can type some text and enter. The text is printed again and the program exits.

Now, if I run through bwrap --ro-bind / / -- /usr/bin/python3 test.py, I get the error:

handle: <Handle interrupt() at test-interrupt.py:7>
Traceback (most recent call last):
  File "/usr/lib/python3.8/asyncio/events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "test-interrupt.py", line 8, in interrupt
    s = input("test")
EOFError

This is because is bwrap is part of foreground process group. Instead it should be the child process as to be the foreground process group.

valentindavid avatar Apr 26 '20 15:04 valentindavid

Coincidentally, I just encountered this issue! I believe bwrap could also just handle the relevant signals and redirect them to the child process?

Something like this:

void forward_signal_to_child(int signal_number) {
    kill(pid, signal_number);
}

...

signal(SIGINT, forward_signal_to_child);
// And others...

Maybe just do this if a flag is passed in, in order to keep both possibilities.

weirdNox avatar Apr 27 '20 19:04 weirdNox

I think creating a pgrp for the child process is better.

To work around this issue I have created a small wrapper for my application: https://gitlab.com/valentindavid/flatpak-buildstream/-/blob/39705ab4451a20bf4caacf061f392beafddad040/files/bst-wrapper.py

valentindavid avatar Apr 27 '20 19:04 valentindavid

I believe bwrap could also just handle the relevant signals and redirect them to the child process?

If I understand it correctly, this would allow to reload some child processes by sending SIGHUP to bwrap. systemctl reload would work almost out of the box (with ExecReload=/usr/bin/kill -HUP $MAINPID). That would be great.

wansing avatar Aug 03 '20 08:08 wansing

If I understand it correctly, this would allow to reload some child processes by sending SIGHUP to bwrap

Only if bwrap specifically catches and forwards SIGHUP - #402 only catches and forwards SIGINT and SIGTERM.

smcv avatar Jun 10 '21 16:06 smcv

@smcv Can you please review either #586 or #588 for this issue? (586 is probably the worse way to go about this, but it was quick to do and solves my annoyance...)

aaruni96 avatar Aug 22 '23 10:08 aaruni96

@smcv Can you please review either #586 or #588 for this issue? (586 is probably the worse way to go about this, but it was quick to do and solves my annoyance...)

Looking into this is on my list, but my list is not short, and everyone wants their item on the list to be my top priority.

smcv avatar Aug 22 '23 11:08 smcv

either #586 or #588

We may even want both, if I'm reading this correctly (?):

  • I'm running a service in bwrap in a systemd-service: That wouldn't have any terminal emulation, right? But I'll still want the service to be able to receive SIGHUP or SIGINT or SIGTERM
  • When I run a program in bwrap in a terminal, then the pgroup trickery would be "more correct" than just forwarding?
  • Or would the pgroup solution also work without a terminal?

bendlas avatar Aug 24 '23 22:08 bendlas