Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logind-user: switch tracking logic to user-runtime-dir@.service #30910

Merged
merged 5 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions src/basic/env-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -310,19 +310,17 @@ char **strv_env_delete(char **x, size_t n_lists, ...) {
return TAKE_PTR(t);
}

char **strv_env_unset(char **l, const char *p) {
char **f, **t;
char** strv_env_unset(char **l, const char *p) {
assert(p);

if (!l)
return NULL;

assert(p);

/* Drops every occurrence of the env var setting p in the
* string list. Edits in-place. */

char **f, **t;
for (f = t = l; *f; f++) {

if (env_match(*f, p)) {
free(*f);
continue;
Expand All @@ -335,27 +333,25 @@ char **strv_env_unset(char **l, const char *p) {
return l;
}

char **strv_env_unset_many(char **l, ...) {
char **f, **t;

char** strv_env_unset_many_internal(char **l, ...) {
if (!l)
return NULL;

/* Like strv_env_unset() but applies many at once. Edits in-place. */

char **f, **t;
for (f = t = l; *f; f++) {
bool found = false;
const char *p;
va_list ap;

va_start(ap, l);

while ((p = va_arg(ap, const char*))) {
while ((p = va_arg(ap, const char*)))
if (env_match(*f, p)) {
found = true;
break;
}
}

va_end(ap);

Expand Down
5 changes: 3 additions & 2 deletions src/basic/env-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ char** _strv_env_merge(char **first, ...);
#define strv_env_merge(first, ...) _strv_env_merge(first, __VA_ARGS__, POINTER_MAX)
char **strv_env_delete(char **x, size_t n_lists, ...); /* New copy */

char **strv_env_unset(char **l, const char *p); /* In place ... */
char **strv_env_unset_many(char **l, ...) _sentinel_;
char** strv_env_unset(char **l, const char *p); /* In place ... */
char** strv_env_unset_many_internal(char **l, ...) _sentinel_;
#define strv_env_unset_many(l, ...) strv_env_unset_many_internal(l, __VA_ARGS__, NULL)
int strv_env_replace_consume(char ***l, char *p); /* In place ... */
int strv_env_replace_strdup(char ***l, const char *assignment);
int strv_env_replace_strdup_passthrough(char ***l, const char *assignment);
Expand Down
17 changes: 14 additions & 3 deletions src/core/manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -641,8 +641,7 @@ static char** sanitize_environment(char **l) {
"TRIGGER_TIMER_REALTIME_USEC",
"TRIGGER_UNIT",
"WATCHDOG_PID",
"WATCHDOG_USEC",
NULL);
"WATCHDOG_USEC");

/* Let's order the environment alphabetically, just to make it pretty */
return strv_sort(l);
Expand All @@ -668,14 +667,26 @@ int manager_default_environment(Manager *m) {
/* Import locale variables LC_*= from configuration */
(void) locale_setup(&m->transient_environment);
} else {
/* The user manager passes its own environment along to its children, except for $PATH. */
/* The user manager passes its own environment along to its children, except for $PATH and
* session envs. */

m->transient_environment = strv_copy(environ);
if (!m->transient_environment)
return log_oom();

r = strv_env_replace_strdup(&m->transient_environment, "PATH=" DEFAULT_USER_PATH);
if (r < 0)
return log_oom();

/* Envvars set for our 'manager' class session are private and should not be propagated
* to children. Also it's likely that the graphical session will set these on their own. */
strv_env_unset_many(m->transient_environment,
"XDG_SESSION_ID",
"XDG_SESSION_CLASS",
"XDG_SESSION_TYPE",
"XDG_SESSION_DESKTOP",
"XDG_SEAT",
"XDG_VTNR");
}

sanitize_environment(m->transient_environment);
Expand Down
43 changes: 26 additions & 17 deletions src/login/logind-dbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -4105,14 +4105,19 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err
user = hashmap_get(m->user_units, unit);
if (user) {
/* If the user is stopping, we're tracking stop jobs here. So don't send reply. */
if (!user->stopping && streq_ptr(path, user->service_job)) {
user->service_job = mfree(user->service_job);

LIST_FOREACH(sessions_by_user, s, user->sessions)
/* Don't propagate user service failures to the client */
session_jobs_reply(s, id, unit, /* error = */ NULL /* don't propagate user service failures to the client */);

user_save(user);
if (!user->stopping) {
char **user_job;
FOREACH_ARGUMENT(user_job, &user->runtime_dir_job, &user->service_manager_job)
if (streq_ptr(path, *user_job)) {
*user_job = mfree(*user_job);

LIST_FOREACH(sessions_by_user, s, user->sessions)
/* Don't propagate user service failures to the client */
session_jobs_reply(s, id, unit, /* error = */ NULL);

user_save(user);
break;
}
}

user_add_to_gc_queue(user);
Expand Down Expand Up @@ -4242,26 +4247,26 @@ int manager_start_scope(
const PidRef *pidref,
const char *slice,
const char *description,
char **wants,
char **after,
const char * const *requires,
const char * const *extra_after,
const char *requires_mounts_for,
sd_bus_message *more_properties,
sd_bus_error *error,
char **job) {
char **ret_job) {

_cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL;
int r;

assert(manager);
assert(scope);
assert(pidref_is_set(pidref));
assert(job);
assert(ret_job);

r = bus_message_new_method_call(manager->bus, &m, bus_systemd_mgr, "StartTransientUnit");
if (r < 0)
return r;

r = sd_bus_message_append(m, "ss", strempty(scope), "fail");
r = sd_bus_message_append(m, "ss", scope, "fail");
if (r < 0)
return r;

Expand All @@ -4281,13 +4286,17 @@ int manager_start_scope(
return r;
}

STRV_FOREACH(i, wants) {
r = sd_bus_message_append(m, "(sv)", "Wants", "as", 1, *i);
STRV_FOREACH(i, requires) {
r = sd_bus_message_append(m, "(sv)", "Requires", "as", 1, *i);
if (r < 0)
return r;

r = sd_bus_message_append(m, "(sv)", "After", "as", 1, *i);
if (r < 0)
return r;
}

STRV_FOREACH(i, after) {
STRV_FOREACH(i, extra_after) {
r = sd_bus_message_append(m, "(sv)", "After", "as", 1, *i);
if (r < 0)
return r;
Expand Down Expand Up @@ -4339,7 +4348,7 @@ int manager_start_scope(
if (r < 0)
return r;

return strdup_job(reply, job);
return strdup_job(reply, ret_job);
}

int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job) {
Expand Down
13 changes: 12 additions & 1 deletion src/login/logind-dbus.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,18 @@ int match_reloading(sd_bus_message *message, void *userdata, sd_bus_error *error

int manager_send_changed(Manager *manager, const char *property, ...) _sentinel_;

int manager_start_scope(Manager *manager, const char *scope, const PidRef *pidref, const char *slice, const char *description, char **wants, char **after, const char *requires_mounts_for, sd_bus_message *more_properties, sd_bus_error *error, char **job);
int manager_start_scope(
Manager *manager,
const char *scope,
const PidRef *pidref,
const char *slice,
const char *description,
const char * const *requires,
const char * const *extra_after,
const char *requires_mounts_for,
sd_bus_message *more_properties,
sd_bus_error *error,
char **ret_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, const char *job_mode, sd_bus_error *error, char **job);
int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error);
Expand Down
29 changes: 18 additions & 11 deletions src/login/logind-session-dbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -426,16 +426,17 @@ static int method_set_class(sd_bus_message *message, void *userdata, sd_bus_erro
/* For now, we'll allow only upgrades user-incomplete → user */
if (class != SESSION_USER)
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
"Class may only be set to 'user', refusing.");
"Class may only be set to 'user'");

if (s->class == SESSION_USER) /* No change, shortcut */
return sd_bus_reply_method_return(message, NULL);
if (s->class != SESSION_USER_INCOMPLETE)
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
"Only sessions with class 'user-incomplete' may change class, refusing.");
"Only sessions with class 'user-incomplete' may change class");

if (s->upgrade_message)
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
"Set session class operation already in progress, refsuing.");
"Set session class operation already in progress");

r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID, &creds);
if (r < 0)
Expand All @@ -450,8 +451,7 @@ static int method_set_class(sd_bus_message *message, void *userdata, sd_bus_erro

session_set_class(s, class);

sd_bus_message_unref(s->upgrade_message);
s->upgrade_message = sd_bus_message_ref(message);
unref_and_replace_full(s->upgrade_message, message, sd_bus_message_ref, sd_bus_message_unref);

r = session_send_upgrade_reply(s, /* error= */ NULL);
if (r < 0)
Expand Down Expand Up @@ -868,13 +868,17 @@ int session_send_lock_all(Manager *m, bool lock) {
return r;
}

static bool session_ready(Session *s) {
static bool session_job_pending(Session *s) {
assert(s);
assert(s->user);

/* Returns true when the session is ready, i.e. all jobs we enqueued for it are done (regardless if successful or not) */
/* Check if we have some jobs enqueued and not finished yet. Each time we get JobRemoved signal about
* relevant units, session_send_create_reply and hence us is called (see match_job_removed).
* Note that we don't care about job result here. */

return !s->scope_job &&
(!SESSION_CLASS_WANTS_SERVICE_MANAGER(s->class) || !s->user->service_job);
return s->scope_job ||
s->user->runtime_dir_job ||
(SESSION_CLASS_WANTS_SERVICE_MANAGER(s->class) && s->user->service_manager_job);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need to check the class here? If the job is there, we might as well wait for it to finish. (It doesn't matter greatly, but the condition "are there any jobs" is easier than "are the any jobs that meeting the following criteria".)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then manager sessions would deadlock, since they'll start waiting for themselves to finish startup.

}

int session_send_create_reply(Session *s, sd_bus_error *error) {
Expand All @@ -890,7 +894,9 @@ int session_send_create_reply(Session *s, sd_bus_error *error) {
if (!s->create_message)
return 0;

if (!sd_bus_error_is_set(error) && !session_ready(s))
/* If error occurred, return it immediately. Otherwise let's wait for all jobs to finish before
* continuing. */
if (!sd_bus_error_is_set(error) && session_job_pending(s))
return 0;

c = TAKE_PTR(s->create_message);
Expand Down Expand Up @@ -938,7 +944,8 @@ int session_send_upgrade_reply(Session *s, sd_bus_error *error) {
if (!s->upgrade_message)
return 0;

if (!sd_bus_error_is_set(error) && !session_ready(s))
/* See comments in session_send_create_reply */
if (!sd_bus_error_is_set(error) && session_job_pending(s))
return 0;

c = TAKE_PTR(s->upgrade_message);
Expand Down
Loading
Loading