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.
This commit is contained in:
Lennart Poettering
parent
a911bb9ab2
commit
5f41d1f10f
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
session_send_signal(s, true);
|
||||
|
||||
if (s->seat) {
|
||||
if (s->seat)
|
||||
seat_save(s->seat);
|
||||
|
||||
/* Send signals */
|
||||
session_send_signal(s, true);
|
||||
user_send_changed(s->user, "Sessions", NULL);
|
||||
if (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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue