151 lines
5.7 KiB
Diff
151 lines
5.7 KiB
Diff
From 75aad3b101548151905d528269ffd2a388955193 Mon Sep 17 00:00:00 2001
|
||
From: Philip Withnall <philip.withnall@collabora.co.uk>
|
||
Date: Fri, 29 May 2015 10:49:21 +0100
|
||
Subject: [PATCH] logind: Fix user_elect_display() to be more stable
|
||
MIME-Version: 1.0
|
||
Content-Type: text/plain; charset=UTF-8
|
||
Content-Transfer-Encoding: 8bit
|
||
|
||
The previous implementation of user_elect_display() could easily end up
|
||
overwriting the user’s valid graphical session with a new TTY session.
|
||
For example, consider the situation where there is one session:
|
||
c1, type = SESSION_X11, !stopping, class = SESSION_USER
|
||
it is initially elected as the user’s display (i.e. u->display = c1).
|
||
|
||
If another session is started, on a different VT, the sessions_by_user
|
||
list becomes:
|
||
c1, type = SESSION_X11, !stopping, class = SESSION_USER
|
||
c2, type = SESSION_TTY, !stopping, class = SESSION_USER
|
||
|
||
In the previous code, graphical = c1 and text = c2, as expected.
|
||
However, neither graphical nor text fulfil the conditions for setting
|
||
u->display = graphical (because neither is better than u->display), so
|
||
the code falls through to check the text variable. The conditions for
|
||
this match, as u->display->type != SESSION_TTY (it’s actually
|
||
SESSION_X11). Hence u->display is set to c2, which is incorrect, because
|
||
session c1 is still valid.
|
||
|
||
Refactor user_elect_display() to use a more explicit filter and
|
||
pre-order comparison over the sessions. This can be demonstrated to be
|
||
stable and only ever ‘upgrade’ the session to a more graphical one.
|
||
|
||
https://bugs.freedesktop.org/show_bug.cgi?id=90769
|
||
(cherry picked from commit 7ffeb45cc63e1326690fd9461b7a4719a3d4f85c)
|
||
---
|
||
src/login/logind-user.c | 90 +++++++++++++++++++++++++++++--------------------
|
||
1 file changed, 54 insertions(+), 36 deletions(-)
|
||
|
||
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
|
||
index 71bff96728..2f62e34f63 100644
|
||
--- a/src/login/logind-user.c
|
||
+++ b/src/login/logind-user.c
|
||
@@ -738,54 +738,72 @@ int user_kill(User *u, int signo) {
|
||
return manager_kill_unit(u->manager, u->slice, KILL_ALL, signo, NULL);
|
||
}
|
||
|
||
+static bool
|
||
+elect_display_filter(Session *s) {
|
||
+ /* Return true if the session is a candidate for the user’s ‘primary
|
||
+ * session’ or ‘display’. */
|
||
+ assert(s);
|
||
+
|
||
+ return (s->class == SESSION_USER && !s->stopping);
|
||
+}
|
||
+
|
||
+static int
|
||
+elect_display_compare(Session *s1, Session *s2) {
|
||
+ /* Indexed by SessionType. Lower numbers mean more preferred. */
|
||
+ const int type_ranks[_SESSION_TYPE_MAX] = {
|
||
+ [SESSION_UNSPECIFIED] = 0,
|
||
+ [SESSION_TTY] = -2,
|
||
+ [SESSION_X11] = -3,
|
||
+ [SESSION_WAYLAND] = -3,
|
||
+ [SESSION_MIR] = -3,
|
||
+ [SESSION_WEB] = -1,
|
||
+ };
|
||
+
|
||
+ /* Calculate the partial order relationship between s1 and s2,
|
||
+ * returning < 0 if s1 is preferred as the user’s ‘primary session’,
|
||
+ * 0 if s1 and s2 are equally preferred or incomparable, or > 0 if s2
|
||
+ * is preferred.
|
||
+ *
|
||
+ * s1 or s2 may be NULL. */
|
||
+ if ((s1 == NULL) != (s2 == NULL))
|
||
+ return (s1 == NULL) - (s2 == NULL);
|
||
+
|
||
+ if (s1->stopping != s2->stopping)
|
||
+ return s1->stopping - s2->stopping;
|
||
+
|
||
+ if ((s1->class != SESSION_USER) != (s2->class != SESSION_USER))
|
||
+ return (s1->class != SESSION_USER) - (s2->class != SESSION_USER);
|
||
+
|
||
+ if ((s1->type == _SESSION_TYPE_INVALID) != (s2->type == _SESSION_TYPE_INVALID))
|
||
+ return (s1->type == _SESSION_TYPE_INVALID) - (s2->type == _SESSION_TYPE_INVALID);
|
||
+
|
||
+ if (s1->type != s2->type)
|
||
+ return type_ranks[s1->type] - type_ranks[s2->type];
|
||
+
|
||
+ return 0;
|
||
+}
|
||
+
|
||
void user_elect_display(User *u) {
|
||
- Session *graphical = NULL, *text = NULL, *other = NULL, *s;
|
||
+ Session *s;
|
||
|
||
assert(u);
|
||
|
||
/* This elects a primary session for each user, which we call
|
||
* the "display". We try to keep the assignment stable, but we
|
||
* "upgrade" to better choices. */
|
||
+ log_debug("Electing new display for user %s", u->name);
|
||
|
||
LIST_FOREACH(sessions_by_user, s, u->sessions) {
|
||
-
|
||
- if (s->class != SESSION_USER)
|
||
- continue;
|
||
-
|
||
- if (s->stopping)
|
||
+ if (!elect_display_filter(s)) {
|
||
+ log_debug("Ignoring session %s", s->id);
|
||
continue;
|
||
+ }
|
||
|
||
- if (SESSION_TYPE_IS_GRAPHICAL(s->type))
|
||
- graphical = s;
|
||
- else if (s->type == SESSION_TTY)
|
||
- text = s;
|
||
- else
|
||
- other = s;
|
||
- }
|
||
-
|
||
- if (graphical &&
|
||
- (!u->display ||
|
||
- u->display->class != SESSION_USER ||
|
||
- u->display->stopping ||
|
||
- !SESSION_TYPE_IS_GRAPHICAL(u->display->type))) {
|
||
- u->display = graphical;
|
||
- return;
|
||
- }
|
||
-
|
||
- if (text &&
|
||
- (!u->display ||
|
||
- u->display->class != SESSION_USER ||
|
||
- u->display->stopping ||
|
||
- u->display->type != SESSION_TTY)) {
|
||
- u->display = text;
|
||
- return;
|
||
+ if (elect_display_compare(s, u->display) < 0) {
|
||
+ log_debug("Choosing session %s in preference to %s", s->id, u->display ? u->display->id : "-");
|
||
+ u->display = s;
|
||
+ }
|
||
}
|
||
-
|
||
- if (other &&
|
||
- (!u->display ||
|
||
- u->display->class != SESSION_USER ||
|
||
- u->display->stopping))
|
||
- u->display = other;
|
||
}
|
||
|
||
static const char* const user_state_table[_USER_STATE_MAX] = {
|