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

[piliu]

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);
+                        }
                 }
         }