<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 7, 2017 at 8:58 AM, Dan Williams <span dir="ltr"><<a href="mailto:dcbw@redhat.com" target="_blank">dcbw@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Sun, 2017-08-06 at 13:13 +0200, Aleksander Morgado wrote:<br>
> If e.g. mbim-proxy is started by ModemManager and we send a Ctrl+C to<br>
> it, the signal would be propagated to the mbim-proxy process and we<br>
> would kill it right away, while leaving ModemManager still around<br>
> wondering why the socket to the proxy got a HUP.<br>
><br>
> Avoid this, by making sure the mbim-proxy gets its own process group.<br></span></blockquote><div><br></div><div>lgtm</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<br>
</span>The way you've done it here is AFAIK the standard way to detach a child<br>
process and make it standalone.  We've done that for years in most of<br>
the things NetworkManager spawns, for example (VPN daemons, DHCP<br>
daemon, avahi-autoipd, dnsmasq, teamd, etc).<br>
<br>
Though instead of (0,0) NM uses:<br>
<br>
        pid_t pid;<br>
<br>
        pid = getpid ();<br>
        setpgid (pid, pid);<br>
<br>
Which I think is a bit more idiomatic.<br></blockquote><div><br></div><div>Isn't that equivalent to `setpgid (0, 0)`? Is `setgpid (pid, pid)` preferred for some reason?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Dan<br>
<div><div class="h5"><br>
> ---<br>
><br>
> Hey,<br>
><br>
> Eric, adding you in CC because I got this issue while testing your<br>
> ModemManager patch to act on proxy crashes. I was running MM in --<br>
> debug mode and sent a Ctrl-C to stop it, and that actually ended up<br>
> killing the proxy underneath and your reprobe logic hitting in... :)<br>
><br>
> What do you guys think? I believe this is reasonable, the proxy<br>
> should really be a complete independent spawn not tied to the process<br>
> which originated it.<br>
><br>
> The same change would be applicable to qmi-proxy.<br>
><br>
> ---<br>
>  src/libmbim-glib/mbim-device.<wbr>c | 9 ++++++++-<br>
>  1 file changed, 8 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/libmbim-glib/mbim-<wbr>device.c b/src/libmbim-glib/mbim-<br>
> device.c<br>
> index 68544af..4b7d431 100644<br>
> --- a/src/libmbim-glib/mbim-<wbr>device.c<br>
> +++ b/src/libmbim-glib/mbim-<wbr>device.c<br>
> @@ -1077,6 +1077,13 @@ wait_for_proxy_cb (GTask *task)<br>
>  }<br>
><br>
>  static void<br>
> +spawn_child_setup (void)<br>
> +{<br>
> +    if (setpgid (0, 0) < 0)<br>
> +        g_warning ("couldn't setup proxy specific process group");<br>
> +}<br>
> +<br>
> +static void<br>
>  create_iochannel_with_socket (GTask *task)<br>
>  {<br>
>      MbimDevice *self;<br>
> @@ -1138,7 +1145,7 @@ create_iochannel_with_socket (GTask *task)<br>
>                              <wbr>argc,<br>
>                              <wbr>NULL, /* envp */<br>
>                              <wbr>G_SPAWN_STDOUT_TO_DEV_NULL |<br>
> G_SPAWN_STDERR_TO_DEV_NULL,<br>
> -                            <wbr>NULL, /* child_setup */<br>
> +                            (<wbr>GSpawnChildSetupFunc)<br>
> spawn_child_setup,<br>
>                              <wbr>NULL, /* child_setup_user_data */<br>
>                              <wbr>NULL,<br>
>                              &<wbr>error)) {<br>
> --<br>
> 2.13.3<br>
</div></div>> ______________________________<wbr>_________________<br>
> libmbim-devel mailing list<br>
> <a href="mailto:libmbim-devel@lists.freedesktop.org">libmbim-devel@lists.<wbr>freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/libmbim-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/libmbim-devel</a><br>
</blockquote></div><br></div></div>