summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2014-02-06 18:32:14 +0100
committerLennart Poettering <lennart@poettering.net>2014-02-07 15:14:36 +0100
commit5f41d1f10fd97e93517b6a762b1bec247f4d1171 (patch)
treea599559b6177bd9fccd01c56f74fad9b81a61851
parenta911bb9ab27ac0eb3bbf4e8b4109e5da9b88eee3 (diff)
logind: rework session shutdown logic
Simplify the shutdown logic a bit: - Keep the session FIFO around in the PAM module, even after the session shutdown hook has been finished. This allows logind to track precisely when the PAM handler goes away. - In the ReleaseSession() call start a timer, that will stop terminate the session when elapsed. - Never fiddle with the KillMode of scopes to configure whether user processes should be killed or not. Instead, simply leave the scope units around when we terminate a session whose processes should not be killed. - When killing is enabled, stop the session scope on FIFO EOF or after the ReleaseSession() timeout. When killing is disabled, simply tell PID 1 to abandon the scope. Because the scopes stay around and hence all processes are always member of a scope, the system shutdown logic should be more robust, as the scopes can be shutdown as part of the usual shutdown logic.
-rw-r--r--src/login/logind-dbus.c51
-rw-r--r--src/login/logind-session.c100
-rw-r--r--src/login/logind-session.h5
-rw-r--r--src/login/logind-user.c23
-rw-r--r--src/login/logind-user.h1
-rw-r--r--src/login/logind.c14
-rw-r--r--src/login/logind.h3
-rw-r--r--src/login/pam-module.c28
8 files changed, 147 insertions, 78 deletions
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 4745961773..40885557c1 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -748,15 +748,7 @@ static int method_release_session(sd_bus *bus, sd_bus_message *message, void *us
if (!session)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_SESSION, "No session '%s' known", name);
- /* We use the FIFO to detect stray sessions where the process
- invoking PAM dies abnormally. We need to make sure that
- that process is not killed if at the clean end of the
- session it closes the FIFO. Hence, with this call
- explicitly turn off the FIFO logic, so that the PAM code
- can finish clean up on its own */
- session_remove_fifo(session);
- session_save(session);
- user_save(session->user);
+ session_release(session);
return sd_bus_reply_method_return(message, NULL);
}
@@ -2185,7 +2177,6 @@ int manager_start_scope(
const char *slice,
const char *description,
const char *after,
- const char *kill_mode,
sd_bus_error *error,
char **job) {
@@ -2232,12 +2223,6 @@ int manager_start_scope(
return r;
}
- if (!isempty(kill_mode)) {
- r = sd_bus_message_append(m, "(sv)", "KillMode", "s", kill_mode);
- if (r < 0)
- return r;
- }
-
/* cgroup empty notification is not available in containers
* currently. To make this less problematic, let's shorten the
* stop timeout for sessions, so that we don't wait
@@ -2372,6 +2357,40 @@ int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, c
return 1;
}
+int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error) {
+ _cleanup_bus_message_unref_ sd_bus_message *reply = NULL;
+ _cleanup_free_ char *path = NULL;
+ int r;
+
+ assert(manager);
+ assert(scope);
+
+ path = unit_dbus_path_from_name(scope);
+ if (!path)
+ return -ENOMEM;
+
+ r = sd_bus_call_method(
+ manager->bus,
+ "org.freedesktop.systemd1",
+ path,
+ "org.freedesktop.systemd1.Scope",
+ "Abandon",
+ error,
+ NULL,
+ NULL);
+ if (r < 0) {
+ if (sd_bus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) ||
+ sd_bus_error_has_name(error, BUS_ERROR_LOAD_FAILED)) {
+ sd_bus_error_free(error);
+ return 0;
+ }
+
+ return r;
+ }
+
+ return 1;
+}
+
int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo, sd_bus_error *error) {
assert(manager);
assert(unit);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index bec59c044b..95105e5325 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -40,6 +40,10 @@
#include "bus-error.h"
#include "logind-session.h"
+#define RELEASE_USEC (20*USEC_PER_SEC)
+
+static void session_remove_fifo(Session *s);
+
static unsigned long devt_hash_func(const void *p, const uint8_t hash_key[HASH_KEY_SIZE]) {
uint64_t u = *(const dev_t*)p;
@@ -103,6 +107,8 @@ void session_free(Session *s) {
if (s->in_gc_queue)
LIST_REMOVE(gc_queue, s->manager->session_gc_queue, s);
+ s->timer_event_source = sd_event_source_unref(s->timer_event_source);
+
session_remove_fifo(s);
session_drop_controller(s);
@@ -147,6 +153,8 @@ void session_free(Session *s) {
hashmap_remove(s->manager->sessions, s->id);
+ s->vt_source = sd_event_source_unref(s->vt_source);
+
free(s->state_file);
free(s);
}
@@ -472,7 +480,6 @@ static int session_start_scope(Session *s) {
if (!s->scope) {
_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_free_ char *description = NULL;
- const char *kill_mode;
char *scope, *job;
description = strjoin("Session ", s->id, " of user ", s->user->name, NULL);
@@ -483,9 +490,7 @@ static int session_start_scope(Session *s) {
if (!scope)
return log_oom();
- kill_mode = manager_shall_kill(s->manager, s->user->name) ? "control-group" : "none";
-
- r = manager_start_scope(s->manager, scope, s->leader, s->user->slice, description, "systemd-user-sessions.service", kill_mode, &error, &job);
+ r = manager_start_scope(s->manager, scope, s->leader, s->user->slice, description, "systemd-logind.service", &error, &job);
if (r < 0) {
log_error("Failed to start session scope %s: %s %s",
scope, bus_error_message(&error, r), error.name);
@@ -541,23 +546,22 @@ int session_start(Session *s) {
s->started = true;
- /* Save session data */
+ /* Save data */
session_save(s);
user_save(s->user);
+ if (s->seat)
+ seat_save(s->seat);
+ /* Send signals */
session_send_signal(s, true);
-
+ user_send_changed(s->user, "Sessions", NULL);
if (s->seat) {
- seat_save(s->seat);
-
if (s->seat->active == s)
seat_send_changed(s->seat, "Sessions", "ActiveSession", NULL);
else
seat_send_changed(s->seat, "Sessions", NULL);
}
- user_send_changed(s->user, "Sessions", NULL);
-
return 0;
}
@@ -571,14 +575,22 @@ static int session_stop_scope(Session *s) {
if (!s->scope)
return 0;
- r = manager_stop_unit(s->manager, s->scope, &error, &job);
- if (r < 0) {
- log_error("Failed to stop session scope: %s", bus_error_message(&error, r));
- return r;
- }
+ if (manager_shall_kill(s->manager, s->user->name)) {
+ r = manager_stop_unit(s->manager, s->scope, &error, &job);
+ if (r < 0) {
+ log_error("Failed to stop session scope: %s", bus_error_message(&error, r));
+ return r;
+ }
- free(s->scope_job);
- s->scope_job = job;
+ free(s->scope_job);
+ s->scope_job = job;
+ } else {
+ r = manager_abandon_scope(s->manager, s->scope, &error);
+ if (r < 0) {
+ log_error("Failed to abandon session scope: %s", bus_error_message(&error, r));
+ return r;
+ }
+ }
return 0;
}
@@ -591,9 +603,16 @@ int session_stop(Session *s) {
if (!s->user)
return -ESTALE;
+ s->timer_event_source = sd_event_source_unref(s->timer_event_source);
+
+ /* We are going down, don't care about FIFOs anymore */
+ session_remove_fifo(s);
+
/* Kill cgroup */
r = session_stop_scope(s);
+ s->stopping = true;
+
session_save(s);
user_save(s->user);
@@ -618,6 +637,8 @@ int session_finalize(Session *s) {
"MESSAGE=Removed session %s.", s->id,
NULL);
+ s->timer_event_source = sd_event_source_unref(s->timer_event_source);
+
/* Kill session devices */
while ((sd = hashmap_first(s->devices)))
session_device_free(sd);
@@ -635,16 +656,36 @@ int session_finalize(Session *s) {
if (s->seat->active == s)
seat_set_active(s->seat, NULL);
- seat_send_changed(s->seat, "Sessions", NULL);
seat_save(s->seat);
+ seat_send_changed(s->seat, "Sessions", NULL);
}
- user_send_changed(s->user, "Sessions", NULL);
user_save(s->user);
+ user_send_changed(s->user, "Sessions", NULL);
return r;
}
+static int release_timeout_callback(sd_event_source *es, uint64_t usec, void *userdata) {
+ Session *s = userdata;
+
+ assert(es);
+ assert(s);
+
+ session_stop(s);
+ return 0;
+}
+
+void session_release(Session *s) {
+ assert(s);
+
+ if (!s->started || s->stopping)
+ return;
+
+ if (!s->timer_event_source)
+ sd_event_add_monotonic(s->manager->event, now(CLOCK_MONOTONIC) + RELEASE_USEC, 0, release_timeout_callback, s, &s->timer_event_source);
+}
+
bool session_is_active(Session *s) {
assert(s);
@@ -820,7 +861,7 @@ int session_create_fifo(Session *s) {
return r;
}
-void session_remove_fifo(Session *s) {
+static void session_remove_fifo(Session *s) {
assert(s);
if (s->fifo_event_source)
@@ -839,8 +880,6 @@ void session_remove_fifo(Session *s) {
}
bool session_check_gc(Session *s, bool drop_not_started) {
- int r;
-
assert(s);
if (drop_not_started && !s->started)
@@ -850,11 +889,7 @@ bool session_check_gc(Session *s, bool drop_not_started) {
return false;
if (s->fifo_fd >= 0) {
- r = pipe_eof(s->fifo_fd);
- if (r < 0)
- return true;
-
- if (r == 0)
+ if (pipe_eof(s->fifo_fd) <= 0)
return true;
}
@@ -880,12 +915,12 @@ void session_add_to_gc_queue(Session *s) {
SessionState session_get_state(Session *s) {
assert(s);
+ if (s->stopping || s->timer_event_source)
+ return SESSION_CLOSING;
+
if (s->scope_job)
return SESSION_OPENING;
- if (s->fifo_fd < 0)
- return SESSION_CLOSING;
-
if (session_is_active(s))
return SESSION_ACTIVE;
@@ -902,7 +937,7 @@ int session_kill(Session *s, KillWho who, int signo) {
}
static int session_open_vt(Session *s) {
- char path[128];
+ char path[sizeof("/dev/tty") + DECIMAL_STR_MAX(s->vtnr)];
if (!s->vtnr)
return -1;
@@ -980,8 +1015,7 @@ void session_restore_vt(Session *s) {
if (vt < 0)
return;
- sd_event_source_unref(s->vt_source);
- s->vt_source = NULL;
+ s->vt_source = sd_event_source_unref(s->vt_source);
ioctl(vt, KDSETMODE, KD_TEXT);
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 7bf1932032..42552bc2bd 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -110,9 +110,12 @@ struct Session {
bool in_gc_queue:1;
bool started:1;
+ bool stopping:1;
sd_bus_message *create_message;
+ sd_event_source *timer_event_source;
+
char *controller;
Hashmap *devices;
@@ -132,10 +135,10 @@ bool session_is_active(Session *s);
int session_get_idle_hint(Session *s, dual_timestamp *t);
void session_set_idle_hint(Session *s, bool b);
int session_create_fifo(Session *s);
-void session_remove_fifo(Session *s);
int session_start(Session *s);
int session_stop(Session *s);
int session_finalize(Session *s);
+void session_release(Session *s);
int session_save(Session *s);
int session_load(Session *s);
int session_kill(Session *s, KillWho who, int signo);
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index bdb6915630..fdbf6e3aa0 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -515,6 +515,8 @@ int user_stop(User *u) {
if (k < 0)
r = k;
+ u->stopping = true;
+
user_save(u);
return r;
@@ -633,22 +635,27 @@ void user_add_to_gc_queue(User *u) {
UserState user_get_state(User *u) {
Session *i;
- bool all_closing = true;
assert(u);
+ if (u->stopping)
+ return USER_CLOSING;
+
if (u->slice_job || u->service_job)
return USER_OPENING;
- LIST_FOREACH(sessions_by_user, i, u->sessions) {
- if (session_is_active(i))
- return USER_ACTIVE;
- if (session_get_state(i) != SESSION_CLOSING)
- all_closing = false;
- }
+ if (u->sessions) {
+ bool all_closing = true;
+
+ LIST_FOREACH(sessions_by_user, i, u->sessions) {
+ if (session_is_active(i))
+ return USER_ACTIVE;
+ if (session_get_state(i) != SESSION_CLOSING)
+ all_closing = false;
+ }
- if (u->sessions)
return all_closing ? USER_CLOSING : USER_ONLINE;
+ }
if (user_check_linger_file(u) > 0)
return USER_LINGERING;
diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 0062880560..b0fefe9b9c 100644
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -61,6 +61,7 @@ struct User {
bool in_gc_queue:1;
bool started:1;
+ bool stopping:1;
LIST_HEAD(Session, sessions);
LIST_FIELDS(User, gc_queue);
diff --git a/src/login/logind.c b/src/login/logind.c
index 48da7b173b..a6f84e8536 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -871,8 +871,15 @@ void manager_gc(Manager *m, bool drop_not_started) {
LIST_REMOVE(gc_queue, m->session_gc_queue, session);
session->in_gc_queue = false;
- if (!session_check_gc(session, drop_not_started)) {
+ /* First, if we are not closing yet, initiate stopping */
+ if (!session_check_gc(session, drop_not_started) &&
+ session_get_state(session) != SESSION_CLOSING)
session_stop(session);
+
+ /* Normally, this should make the session busy again,
+ * if it doesn't then let's get rid of it
+ * immediately */
+ if (!session_check_gc(session, drop_not_started)) {
session_finalize(session);
session_free(session);
}
@@ -882,8 +889,11 @@ void manager_gc(Manager *m, bool drop_not_started) {
LIST_REMOVE(gc_queue, m->user_gc_queue, user);
user->in_gc_queue = false;
- if (!user_check_gc(user, drop_not_started)) {
+ if (!user_check_gc(user, drop_not_started) &&
+ user_get_state(user) != USER_CLOSING)
user_stop(user);
+
+ if (!user_check_gc(user, drop_not_started)) {
user_finalize(user);
user_free(user);
}
diff --git a/src/login/logind.h b/src/login/logind.h
index b84137c78b..c1a5b6a80f 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -162,9 +162,10 @@ int manager_send_changed(Manager *manager, const char *property, ...) _sentinel_
int manager_dispatch_delayed(Manager *manager);
-int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, const char *after, const char *kill_mode, sd_bus_error *error, char **job);
+int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, const char *after, sd_bus_error *error, char **job);
int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job);
int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job);
+int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error);
int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo, sd_bus_error *error);
int manager_unit_is_active(Manager *manager, const char *unit);
int manager_job_is_active(Manager *manager, const char *path);
diff --git a/src/login/pam-module.c b/src/login/pam-module.c
index 3b2966b30c..79a9042ffd 100644
--- a/src/login/pam-module.c
+++ b/src/login/pam-module.c
@@ -499,7 +499,7 @@ _public_ PAM_EXTERN int pam_sm_close_session(
_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_bus_unref_ sd_bus *bus = NULL;
- const void *p = NULL, *existing = NULL;
+ const void *existing = NULL;
const char *id;
int r;
@@ -519,10 +519,8 @@ _public_ PAM_EXTERN int pam_sm_close_session(
r = sd_bus_open_system(&bus);
if (r < 0) {
- pam_syslog(handle, LOG_ERR,
- "Failed to connect to system bus: %s", strerror(-r));
- r = PAM_SESSION_ERR;
- goto finish;
+ pam_syslog(handle, LOG_ERR, "Failed to connect to system bus: %s", strerror(-r));
+ return PAM_SESSION_ERR;
}
r = sd_bus_call_method(bus,
@@ -535,20 +533,16 @@ _public_ PAM_EXTERN int pam_sm_close_session(
"s",
id);
if (r < 0) {
- pam_syslog(handle, LOG_ERR,
- "Failed to release session: %s", bus_error_message(&error, r));
-
- r = PAM_SESSION_ERR;
- goto finish;
+ pam_syslog(handle, LOG_ERR, "Failed to release session: %s", bus_error_message(&error, r));
+ return PAM_SESSION_ERR;
}
}
- r = PAM_SUCCESS;
+ /* Note that we are knowingly leaking the FIFO fd here. This
+ * way, logind can watch us die. If we closed it here it would
+ * not have any clue when that is completed. Given that one
+ * cannot really have multiple PAM sessions open from the same
+ * process this means we will leak one FD at max. */
-finish:
- pam_get_data(handle, "systemd.session-fd", &p);
- if (p)
- close_nointr(PTR_TO_INT(p) - 1);
-
- return r;
+ return PAM_SUCCESS;
}