[systemd-devel] [PATCH] Fix timeout when stopping Type=notify service

[Andrey Borzenkov]

В Fri, 20 Sep 2013 22:53:52 +0200
Olivier Brunel <jjk at jjacky.com> пишет:

> Since 41efeaec a call to service_unwatch_main_pid() is done from
> service_set_main_pid(), which is called upon receiving message MAINPID=
> 
> This had the side effect of not watching pid anymore, and would result in a
> useless timeout when stopping the service, as the unit wouldn't be identified
> from the pid, so not marked stopped which would result in systemd thinking this
> was a timeout.

I confirm that this fixed timeout stopping user manager during shutdown
I observed on openSUSE 13.1 Beta1:
https://bugzilla.novell.com/show_bug.cgi?id=841544 

> ---
> I'm not exactly familiar with systemd's internals, so this might not be the
> correct way to fix this, please correct me if it isn't.
> 
>  src/core/service.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/core/service.c b/src/core/service.c
> index 246a86e..1dccdff 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -3388,9 +3388,17 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags) {
>                          log_warning_unit(u->id,
>                                           "Failed to parse notification message %s", e);
>                  else {
> +                        int r;
> +
>                          log_debug_unit(u->id,
>                                         "%s: got %s", u->id, e);
>                          service_set_main_pid(s, pid);

You probably should sanity check return status and proceed with
unit_watch_pid only if it succeeded. It is not clear what should be
done if it failed - it means service returned bogus value in
notification message, in which case the only logical step is to stop
it as it cannot be trusted anymore? 

> +                        r = unit_watch_pid(u, pid);
> +                        if (r < 0)
> +                                /* FIXME: we need to do something here */
> +                                log_warning_unit(u->id,
> +                                                 "Failed to watch PID %lu from service %s",
> +                                                 (unsigned long) pid, u->id);
>                  }
>          }
>