[PATCH] device: avoid signals sent to the mbim-proxy process

[Dan Williams]

On Sun, 2017-08-06 at 13:13 +0200, Aleksander Morgado wrote:
> If e.g. mbim-proxy is started by ModemManager and we send a Ctrl+C to
> it, the signal would be propagated to the mbim-proxy process and we
> would kill it right away, while leaving ModemManager still around
> wondering why the socket to the proxy got a HUP.
> 
> Avoid this, by making sure the mbim-proxy gets its own process group.

The way you've done it here is AFAIK the standard way to detach a child
process and make it standalone.  We've done that for years in most of
the things NetworkManager spawns, for example (VPN daemons, DHCP
daemon, avahi-autoipd, dnsmasq, teamd, etc).

Though instead of (0,0) NM uses:

        pid_t pid;

        pid = getpid ();
        setpgid (pid, pid);

Which I think is a bit more idiomatic.

Dan

> ---
> 
> Hey,
> 
> Eric, adding you in CC because I got this issue while testing your
> ModemManager patch to act on proxy crashes. I was running MM in --
> debug mode and sent a Ctrl-C to stop it, and that actually ended up
> killing the proxy underneath and your reprobe logic hitting in... :)
> 
> What do you guys think? I believe this is reasonable, the proxy
> should really be a complete independent spawn not tied to the process
> which originated it.
> 
> The same change would be applicable to qmi-proxy.
> 
> ---
>  src/libmbim-glib/mbim-device.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libmbim-glib/mbim-device.c b/src/libmbim-glib/mbim-
> device.c
> index 68544af..4b7d431 100644
> --- a/src/libmbim-glib/mbim-device.c
> +++ b/src/libmbim-glib/mbim-device.c
> @@ -1077,6 +1077,13 @@ wait_for_proxy_cb (GTask *task)
>  }
> 
>  static void
> +spawn_child_setup (void)
> +{
> +    if (setpgid (0, 0) < 0)
> +        g_warning ("couldn't setup proxy specific process group");
> +}
> +
> +static void
>  create_iochannel_with_socket (GTask *task)
>  {
>      MbimDevice *self;
> @@ -1138,7 +1145,7 @@ create_iochannel_with_socket (GTask *task)
>                              argc,
>                              NULL, /* envp */
>                              G_SPAWN_STDOUT_TO_DEV_NULL |
> G_SPAWN_STDERR_TO_DEV_NULL,
> -                            NULL, /* child_setup */
> +                            (GSpawnChildSetupFunc)
> spawn_child_setup,
>                              NULL, /* child_setup_user_data */
>                              NULL,
>                              &error)) {
> --
> 2.13.3
> _______________________________________________
> libmbim-devel mailing list
> libmbim-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/libmbim-devel