[systemd-devel] Is it a bug of manager->n_on_console?

[piliu]

When trying to interact with a emergency shell, this bug brights trouble
by dumping endless status message to console

On 12/25/2017 01:31 PM, piliu wrote:
> Hi,
> 
> When debugging with a shell, manager_status_printf() always prints "A
> start job...". After tracing the systemd's code, I found in that
> function, the cond "if (type == STATUS_TYPE_EPHEMERAL && m &&
> m->n_on_console > 0)" does not meet.
> 
> With a debugging patch, I got the following message, which indicates
> that there is a reference count bug with n_on_console.1
> The debugging message:
> Manager:0x55e55d91d110, unit: dracut pre-pivot and cleanup hook
> n_on_console++, >1
> service_set_state Manager:0x55e55d91d110, unit: dracut pre-pivot and
> cleanup hook n_on_console--, >0
> Manager:0x55e55d91d110, unit: Kdump Vmcore Save Service n_on_console++, >1
> Manager:0x55e55d91d110, unit: Kdump Vmcore Save Service n_on_console--, >0
> Manager:0x55e55d91d110, unit: dracut pre-pivot and cleanup hook
> n_on_console--, >-1
> Manager:0x55e55d91d110, unit: Kdump Emergency n_on_console++, >0
> Manager:0x55e55d91d110, unit: Kdump Emergency n_on_console--, >-1
> Manager:0x55e55d91d110, unit: Kdump Error Handler n_on_console++, >0
> 
> Where you can see "dracut pre-pivot and cleanup hook" dec the refcnt
> twice. (Note: n_on_console is declared as unsigned, so here the negative
> value reflects the underflow.)
> 
> Thanks and regards,
> Pingfan
> 
> --- The patch I used ---
> diff --git a/src/core/service.c b/src/core/service.c
> index ceed1cc..9ea8ecd 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -868,6 +868,8 @@ static void service_set_state(Service *s,
> ServiceState state) {
>                          Manager *m = UNIT(s)->manager;
> 
>                          m->n_on_console --;
> +                        log_debug("service_set_state Manager:0x%lx,
> unit: %s n_on_console--, >%d\n",
> +                                           (unsigned
> long)m,unit_description(UNIT(s)),m->n_on_console);
>                          if (m->n_on_console == 0)
>                                  /* unset no_console_output flag, since
> the console is free */
>                                  m->no_console_output = false;
> diff --git a/src/core/unit.c b/src/core/unit.c
> index 8c0fde8..6c1f21f 100644
> --- a/src/core/unit.c
> +++ b/src/core/unit.c
> @@ -1827,12 +1827,16 @@ void unit_notify(Unit *u, UnitActiveState os,
> UnitActiveState ns, bool reload_su
>                  if (ec && exec_context_may_touch_console(ec)) {
>                          if (UNIT_IS_INACTIVE_OR_FAILED(ns)) {
>                                  m->n_on_console --;
> -
> +                                log_debug("Manager:0x%lx, unit: %s
> n_on_console--, >%d\n",
> +                                                    (unsigned long)m,
> unit_description(u),m->n_on_console);
>                                  if (m->n_on_console == 0)
>                                          /* unset no_console_output
> flag, since the console is free */
>                                          m->no_console_output = false;
> -                        } else
> +                        } else{
>                                  m->n_on_console ++;
> +                                log_debug("Manager:0x%lx, unit: %s
> n_on_console++, >%d\n",
> +                                                    (unsigned long)m,
> unit_description(u),m->n_on_console);
> +                        }
>                  }
>          }
> 
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>