diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c index d695106658b..b3df8cd893c 100644 --- a/src/core/core-varlink.c +++ b/src/core/core-varlink.c @@ -142,7 +142,7 @@ static int vl_method_subscribe_managed_oom_cgroups( /* We only take one subscriber for this method so return an error if there's already an existing one. * This shouldn't happen since systemd-oomd is the only client of this method. */ if (FLAGS_SET(flags, VARLINK_METHOD_MORE) && m->managed_oom_varlink_request) - return varlink_error(m->managed_oom_varlink_request, VARLINK_ERROR_SUBSCRIPTION_TAKEN, NULL); + return varlink_error(link, VARLINK_ERROR_SUBSCRIPTION_TAKEN, NULL); r = json_build(&arr, JSON_BUILD_EMPTY_ARRAY); if (r < 0) @@ -188,6 +188,7 @@ static int vl_method_subscribe_managed_oom_cgroups( if (!FLAGS_SET(flags, VARLINK_METHOD_MORE)) return varlink_reply(link, v); + assert(!m->managed_oom_varlink_request); m->managed_oom_varlink_request = varlink_ref(link); return varlink_notify(m->managed_oom_varlink_request, v); } @@ -475,8 +476,7 @@ void manager_varlink_done(Manager *m) { assert(m); /* Send the final message if we still have a subscribe request open. */ - if (m->managed_oom_varlink_request) - m->managed_oom_varlink_request = varlink_close_unref(m->managed_oom_varlink_request); + m->managed_oom_varlink_request = varlink_close_unref(m->managed_oom_varlink_request); m->varlink_server = varlink_server_unref(m->varlink_server); } diff --git a/src/oom/oomd.c b/src/oom/oomd.c index 674d53fdcfe..6e2a5889d1e 100644 --- a/src/oom/oomd.c +++ b/src/oom/oomd.c @@ -170,7 +170,7 @@ static int run(int argc, char *argv[]) { notify_msg = notify_start(NOTIFY_READY, NOTIFY_STOPPING); - log_info("systemd-oomd starting%s!", arg_dry_run ? " in dry run mode" : ""); + log_debug("systemd-oomd started%s.", arg_dry_run ? " in dry run mode" : ""); r = sd_event_loop(m->event); if (r < 0) diff --git a/src/shared/varlink.c b/src/shared/varlink.c index 31128e02e06..6ed72075ba5 100644 --- a/src/shared/varlink.c +++ b/src/shared/varlink.c @@ -1206,8 +1206,9 @@ int varlink_close(Varlink *v) { varlink_set_state(v, VARLINK_DISCONNECTED); - /* Let's take a reference first, since varlink_detach_server() might drop the final (dangling) ref - * which would destroy us before we can call varlink_clear() */ + /* Let's take a reference first, since varlink_detach_server() might drop the final ref from the + * disconnect callback, which would invalidate the pointer we are holding before we can call + * varlink_clear(). */ varlink_ref(v); varlink_detach_server(v); varlink_clear(v); @@ -1220,17 +1221,33 @@ Varlink* varlink_close_unref(Varlink *v) { if (!v) return NULL; - (void) varlink_close(v); + /* A reference is given to us to be destroyed. But when calling varlink_close(), a callback might + * also drop a reference. We allow this, and will hold a temporary reference to the object to make + * sure that the object still exists when control returns to us. If there's just one reference + * remaining after varlink_close(), even though there were at least two right before, we'll handle + * that gracefully instead of crashing. + * + * In other words, this call drops the donated reference, but if the internal call to varlink_close() + * dropped a reference to, we don't drop the reference afain. This allows the caller to say: + * global_object->varlink = varlink_close_unref(global_object->varlink); + * even though there is some callback which has access to global_object and may drop the reference + * stored in global_object->varlink. Without this step, the same code would have to be written as: + * Varlink *t = TAKE_PTR(global_object->varlink); + * varlink_close_unref(t); + */ + /* n_ref >= 1 */ + varlink_ref(v); /* n_ref >= 2 */ + varlink_close(v); /* n_ref >= 1 */ + if (v->n_ref > 1) + v->n_ref--; /* n_ref >= 1 */ return varlink_unref(v); } Varlink* varlink_flush_close_unref(Varlink *v) { - if (!v) - return NULL; + if (v) + varlink_flush(v); - (void) varlink_flush(v); - (void) varlink_close(v); - return varlink_unref(v); + return varlink_close_unref(v); } static int varlink_enqueue_json(Varlink *v, JsonVariant *m) {