Discussion:
Is it a bug of manager->n_on_console?
Add Reply
piliu
2017-12-25 05:31:12 UTC
Reply
Permalink
Raw Message
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);
+ }
}
}
piliu
2017-12-27 04:14:59 UTC
Reply
Permalink
Raw Message
When trying to interact with a emergency shell, this bug brights trouble
by dumping endless status message to console
Post by 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
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
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Loading...