From 1ac6c613ef11a9ee9c36b25133cc302c4be1a858 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
(cherry picked from commit adefcf2821a386184991054ed2bb6dacc90d7419)

Resolves: #1524359
---
 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 88d156e8fb..4c87ad8a2f 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3379,6 +3379,21 @@ ManagerState manager_state(Manager *m) {
         return MANAGER_RUNNING;
 }
 
+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 b0e4cad1fc..cfc564dfb6 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -379,5 +379,8 @@ const char *manager_get_runtime_prefix(Manager *m);
 
 ManagerState manager_state(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 8a8f4be149..ea71c9e237 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -914,26 +914,6 @@ static void service_set_state(Service *s, ServiceState state) {
         if (state == SERVICE_EXITED && UNIT(s)->manager->n_reloading <= 0)
                 unit_destroy_cgroup_if_empty(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)->id, "%s changed %s -> %s", UNIT(s)->id, service_state_to_string(old_state), service_state_to_string(state));
 
diff --git a/src/core/unit.c b/src/core/unit.c
index 48358bc026..294c9eb70f 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -544,6 +544,9 @@ void unit_free(Unit *u) {
                 u->manager->n_in_gc_queue--;
         }
 
+        if (u->on_console)
+                manager_unref_console(u->manager);
+
         condition_free_list(u->conditions);
         condition_free_list(u->asserts);
 
@@ -1741,6 +1744,23 @@ void unit_trigger_notify(Unit *u) {
                         UNIT_VTABLE(other)->trigger_notify(other, u);
 }
 
+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;
@@ -1784,24 +1804,7 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
         if (UNIT_IS_INACTIVE_OR_FAILED(ns))
                 unit_destroy_cgroup_if_empty(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 fa7de11645..719fc95260 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -238,6 +238,7 @@ struct Unit {
         bool no_gc:1;
 
         bool in_audit:1;
+        bool on_console:1;
 
         bool cgroup_realized:1;
         bool cgroup_members_mask_valid:1;