From adefcf2821a386184991054ed2bb6dacc90d7419 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 24 Jan 2018 19:59:55 +0100
Subject: [PATCH] core: rework how we count the n_on_console counter

Let's add a per-unit boolean that tells us whether our unit is currently
counted or not. This way it's unlikely we get out of sync again and
things are generally more robust.

This also allows us to remove the counting logic specific to service
units (which was in fact mostly a copy from the generic implementation),
in favour of fully generic code.

Replaces: #7824
---
 src/core/manager.c | 15 +++++++++++++++
 src/core/manager.h |  3 +++
 src/core/service.c | 20 --------------------
 src/core/unit.c    | 39 +++++++++++++++++++++------------------
 src/core/unit.h    |  1 +
 5 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index b58f3451d7..f5c89b0414 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -4072,6 +4072,21 @@ char *manager_taint_string(Manager *m) {
         return buf;
 }
 
+void manager_ref_console(Manager *m) {
+        assert(m);
+
+        m->n_on_console++;
+}
+
+void manager_unref_console(Manager *m) {
+
+        assert(m->n_on_console > 0);
+        m->n_on_console--;
+
+        if (m->n_on_console == 0)
+                m->no_console_output = false; /* unset no_console_output flag, since the console is definitely free now */
+}
+
 static const char *const manager_state_table[_MANAGER_STATE_MAX] = {
         [MANAGER_INITIALIZING] = "initializing",
         [MANAGER_STARTING] = "starting",
diff --git a/src/core/manager.h b/src/core/manager.h
index 90d5258b53..2bfcbd559b 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -452,6 +452,9 @@ void manager_deserialize_gid_refs_one(Manager *m, const char *value);
 
 char *manager_taint_string(Manager *m);
 
+void manager_ref_console(Manager *m);
+void manager_unref_console(Manager *m);
+
 const char *manager_state_to_string(ManagerState m) _const_;
 ManagerState manager_state_from_string(const char *s) _pure_;
 
diff --git a/src/core/service.c b/src/core/service.c
index 425e2a1a3c..6476dc68c5 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1060,26 +1060,6 @@ static void service_set_state(Service *s, ServiceState state) {
         if (state == SERVICE_EXITED && !MANAGER_IS_RELOADING(UNIT(s)->manager))
                 unit_prune_cgroup(UNIT(s));
 
-        /* For remain_after_exit services, let's see if we can "release" the
-         * hold on the console, since unit_notify() only does that in case of
-         * change of state */
-        if (state == SERVICE_EXITED &&
-            s->remain_after_exit &&
-            UNIT(s)->manager->n_on_console > 0) {
-
-                ExecContext *ec;
-
-                ec = unit_get_exec_context(UNIT(s));
-                if (ec && exec_context_may_touch_console(ec)) {
-                        Manager *m = UNIT(s)->manager;
-
-                        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;
-                }
-        }
-
         if (old_state != state)
                 log_unit_debug(UNIT(s), "Changed %s -> %s", service_state_to_string(old_state), service_state_to_string(state));
 
diff --git a/src/core/unit.c b/src/core/unit.c
index 4081db8351..932f05baa2 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -630,6 +630,9 @@ void unit_free(Unit *u) {
         if (u->in_cgroup_empty_queue)
                 LIST_REMOVE(cgroup_empty_queue, u->manager->cgroup_empty_queue, u);
 
+        if (u->on_console)
+                manager_unref_console(u->manager);
+
         unit_release_cgroup(u);
 
         if (!MANAGER_IS_RELOADING(u->manager))
@@ -2305,6 +2308,23 @@ finish:
 
 }
 
+static void unit_update_on_console(Unit *u) {
+        bool b;
+
+        assert(u);
+
+        b = unit_needs_console(u);
+        if (u->on_console == b)
+                return;
+
+        u->on_console = b;
+        if (b)
+                manager_ref_console(u->manager);
+        else
+                manager_unref_console(u->manager);
+
+}
+
 void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_success) {
         Manager *m;
         bool unexpected;
@@ -2345,24 +2365,7 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
                 unit_unlink_state_files(u);
         }
 
-        /* Note that this doesn't apply to RemainAfterExit services exiting
-         * successfully, since there's no change of state in that case. Which is
-         * why it is handled in service_set_state() */
-        if (UNIT_IS_INACTIVE_OR_FAILED(os) != UNIT_IS_INACTIVE_OR_FAILED(ns)) {
-                ExecContext *ec;
-
-                ec = unit_get_exec_context(u);
-                if (ec && exec_context_may_touch_console(ec)) {
-                        if (UNIT_IS_INACTIVE_OR_FAILED(ns)) {
-                                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
-                                m->n_on_console++;
-                }
-        }
+        unit_update_on_console(u);
 
         if (u->job) {
                 unexpected = false;
diff --git a/src/core/unit.h b/src/core/unit.h
index ec171ada31..8c79d4ed2e 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -340,6 +340,7 @@ struct Unit {
         bool sent_dbus_new_signal:1;
 
         bool in_audit:1;
+        bool on_console:1;
 
         bool cgroup_realized:1;
         bool cgroup_members_mask_valid:1;