[PATCH] device: avoid signals sent to the mbim-proxy process
Ben Chan
benchan at chromium.org
Mon Aug 7 18:58:50 UTC 2017
On Mon, Aug 7, 2017 at 8:58 AM, Dan Williams <dcbw at redhat.com> wrote:
> 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.
>
lgtm
>
> 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.
>
Isn't that equivalent to `setpgid (0, 0)`? Is `setgpid (pid, pid)`
preferred for some reason?
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/libmbim-devel/attachments/20170807/476f0826/attachment.html>
More information about the libmbim-devel
mailing list