diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 2d65fdbdb8..1aa8c00512 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -1295,7 +1295,7 @@ static int dump(int argc, char *argv[], void *userdata) { &reply, NULL); if (r < 0) - return log_error_errno(r, "Failed issue method call: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to issue method call: %s", bus_error_message(&error, r)); r = sd_bus_message_read(reply, "s", &text); if (r < 0) diff --git a/src/core/automount.c b/src/core/automount.c index c191336c0e..01a6ff806e 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -965,13 +965,16 @@ static const char *automount_sub_state_to_string(Unit *u) { return automount_state_to_string(AUTOMOUNT(u)->state); } -static bool automount_check_gc(Unit *u) { +static bool automount_may_gc(Unit *u) { + Unit *t; + assert(u); - if (!UNIT_TRIGGER(u)) - return false; + t = UNIT_TRIGGER(u); + if (!t) + return true; - return UNIT_VTABLE(UNIT_TRIGGER(u))->check_gc(UNIT_TRIGGER(u)); + return UNIT_VTABLE(t)->may_gc(t); } static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, void *userdata) { @@ -1124,7 +1127,7 @@ const UnitVTable automount_vtable = { .active_state = automount_active_state, .sub_state_to_string = automount_sub_state_to_string, - .check_gc = automount_check_gc, + .may_gc = automount_may_gc, .trigger_notify = automount_trigger_notify, diff --git a/src/core/job.c b/src/core/job.c index c6de8d27e4..a653694d32 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -312,11 +312,13 @@ void job_dump(Job *j, FILE*f, const char *prefix) { "%s-> Job %u:\n" "%s\tAction: %s -> %s\n" "%s\tState: %s\n" - "%s\tIrreversible: %s\n", + "%s\tIrreversible: %s\n" + "%s\tMay GC: %s\n", prefix, j->id, prefix, j->unit->id, job_type_to_string(j->type), prefix, job_state_to_string(j->state), - prefix, yes_no(j->irreversible)); + prefix, yes_no(j->irreversible), + prefix, yes_no(job_may_gc(j))); } /* @@ -1272,7 +1274,7 @@ int job_get_timeout(Job *j, usec_t *timeout) { return 1; } -bool job_check_gc(Job *j) { +bool job_may_gc(Job *j) { Unit *other; Iterator i; void *v; @@ -1280,13 +1282,14 @@ bool job_check_gc(Job *j) { assert(j); /* Checks whether this job should be GC'ed away. We only do this for jobs of units that have no effect on their - * own and just track external state. For now the only unit type that qualifies for this are .device units. */ + * own and just track external state. For now the only unit type that qualifies for this are .device units. + * Returns true if the job can be collected. */ if (!UNIT_VTABLE(j->unit)->gc_jobs) - return true; + return false; if (sd_bus_track_count(j->bus_track) > 0) - return true; + return false; /* FIXME: So this is a bit ugly: for now we don't properly track references made via private bus connections * (because it's nasty, as sd_bus_track doesn't apply to it). We simply remember that the job was once @@ -1296,11 +1299,11 @@ bool job_check_gc(Job *j) { if (set_isempty(j->unit->manager->private_buses)) j->ref_by_private_bus = false; else - return true; + return false; } if (j->type == JOB_NOP) - return true; + return false; /* If a job is ordered after ours, and is to be started, then it needs to wait for us, regardless if we stop or * start, hence let's not GC in that case. */ @@ -1312,7 +1315,7 @@ bool job_check_gc(Job *j) { continue; if (IN_SET(other->job->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) - return true; + return false; } /* If we are going down, but something else is ordered After= us, then it needs to wait for us */ @@ -1324,7 +1327,7 @@ bool job_check_gc(Job *j) { if (other->job->ignore_order) continue; - return true; + return false; } /* The logic above is kinda the inverse of the job_is_runnable() logic. Specifically, if the job "we" is @@ -1344,7 +1347,7 @@ bool job_check_gc(Job *j) { * */ - return false; + return true; } void job_add_to_gc_queue(Job *j) { @@ -1353,7 +1356,7 @@ void job_add_to_gc_queue(Job *j) { if (j->in_gc_queue) return; - if (job_check_gc(j)) + if (!job_may_gc(j)) return; LIST_PREPEND(gc_queue, j->unit->manager->gc_job_queue, j); diff --git a/src/core/job.h b/src/core/job.h index a2f3b7b5d2..2edb63169c 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -233,7 +233,7 @@ void job_shutdown_magic(Job *j); int job_get_timeout(Job *j, usec_t *timeout) _pure_; -bool job_check_gc(Job *j); +bool job_may_gc(Job *j); void job_add_to_gc_queue(Job *j); int job_get_before(Job *j, Job*** ret); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 4c520b8669..c4f91fb262 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1859,7 +1859,7 @@ int config_parse_socket_service( return -ENOEXEC; } - unit_ref_set(&s->service, x); + unit_ref_set(&s->service, UNIT(s), x); return 0; } diff --git a/src/core/manager.c b/src/core/manager.c index 9f8153f1e7..a368fa6b56 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1038,7 +1038,7 @@ static void unit_gc_sweep(Unit *u, unsigned gc_marker) { if (u->in_cleanup_queue) goto bad; - if (unit_check_gc(u)) + if (!unit_may_gc(u)) goto good; u->gc_marker = gc_marker + GC_OFFSET_IN_PATH; @@ -1055,6 +1055,20 @@ static void unit_gc_sweep(Unit *u, unsigned gc_marker) { is_bad = false; } + if (u->refs_by_target) { + const UnitRef *ref; + + LIST_FOREACH(refs_by_target, ref, u->refs_by_target) { + unit_gc_sweep(ref->source, gc_marker); + + if (ref->source->gc_marker == gc_marker + GC_OFFSET_GOOD) + goto good; + + if (ref->source->gc_marker != gc_marker + GC_OFFSET_BAD) + is_bad = false; + } + } + if (is_bad) goto bad; @@ -1125,7 +1139,7 @@ static unsigned manager_dispatch_gc_job_queue(Manager *m) { n++; - if (job_check_gc(j)) + if (!job_may_gc(j)) continue; log_unit_debug(j->unit, "Collecting job."); diff --git a/src/core/mount.c b/src/core/mount.c index 4d80918b89..cfe8ec9044 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1233,12 +1233,15 @@ _pure_ static const char *mount_sub_state_to_string(Unit *u) { return mount_state_to_string(MOUNT(u)->state); } -_pure_ static bool mount_check_gc(Unit *u) { +_pure_ static bool mount_may_gc(Unit *u) { Mount *m = MOUNT(u); assert(m); - return m->from_proc_self_mountinfo; + if (m->from_proc_self_mountinfo) + return false; + + return true; } static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { @@ -1994,7 +1997,7 @@ const UnitVTable mount_vtable = { .active_state = mount_active_state, .sub_state_to_string = mount_sub_state_to_string, - .check_gc = mount_check_gc, + .may_gc = mount_may_gc, .sigchld_event = mount_sigchld_event, diff --git a/src/core/service.c b/src/core/service.c index ab4626f51b..f2d4333719 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2683,7 +2683,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, if (r < 0) log_unit_debug_errno(u, r, "Failed to load accept-socket unit: %s", value); else { - unit_ref_set(&s->accept_socket, socket); + unit_ref_set(&s->accept_socket, u, socket); SOCKET(socket)->n_connections++; } @@ -2832,20 +2832,20 @@ static const char *service_sub_state_to_string(Unit *u) { return service_state_to_string(SERVICE(u)->state); } -static bool service_check_gc(Unit *u) { +static bool service_may_gc(Unit *u) { Service *s = SERVICE(u); assert(s); /* Never clean up services that still have a process around, even if the service is formally dead. Note that - * unit_check_gc() already checked our cgroup for us, we just check our two additional PIDs, too, in case they + * unit_may_gc() already checked our cgroup for us, we just check our two additional PIDs, too, in case they * have moved outside of the cgroup. */ if (main_pid_good(s) > 0 || control_pid_good(s) > 0) - return true; + return false; - return false; + return true; } static int service_retry_pid_file(Service *s) { @@ -3769,7 +3769,7 @@ int service_set_socket_fd(Service *s, int fd, Socket *sock, bool selinux_context s->socket_fd = fd; s->socket_fd_selinux_context_net = selinux_context_net; - unit_ref_set(&s->accept_socket, UNIT(sock)); + unit_ref_set(&s->accept_socket, UNIT(s), UNIT(sock)); return 0; } @@ -3936,7 +3936,7 @@ const UnitVTable service_vtable = { .will_restart = service_will_restart, - .check_gc = service_check_gc, + .may_gc = service_may_gc, .sigchld_event = service_sigchld_event, diff --git a/src/core/slice.c b/src/core/slice.c index ef2177279a..2228669e00 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -76,7 +76,7 @@ static int slice_add_parent_slice(Slice *s) { if (r < 0) return r; - unit_ref_set(&u->slice, parent); + unit_ref_set(&u->slice, u, parent); return 0; } diff --git a/src/core/socket.c b/src/core/socket.c index 38ad8820be..1a57cf0e11 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -250,7 +250,7 @@ int socket_instantiate_service(Socket *s) { if (r < 0) return r; - unit_ref_set(&s->service, u); + unit_ref_set(&s->service, UNIT(s), u); return unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, u, false, UNIT_DEPENDENCY_IMPLICIT); } @@ -377,7 +377,7 @@ static int socket_add_extras(Socket *s) { if (r < 0) return r; - unit_ref_set(&s->service, x); + unit_ref_set(&s->service, u, x); } r = unit_add_two_dependencies(u, UNIT_BEFORE, UNIT_TRIGGERS, UNIT_DEREF(s->service), true, UNIT_DEPENDENCY_IMPLICIT); @@ -2818,12 +2818,12 @@ SocketType socket_port_type_from_string(const char *s) { return _SOCKET_TYPE_INVALID; } -_pure_ static bool socket_check_gc(Unit *u) { +_pure_ static bool socket_may_gc(Unit *u) { Socket *s = SOCKET(u); assert(u); - return s->n_connections > 0; + return s->n_connections == 0; } static int socket_accept_do(Socket *s, int fd) { @@ -3323,7 +3323,7 @@ const UnitVTable socket_vtable = { .active_state = socket_active_state, .sub_state_to_string = socket_sub_state_to_string, - .check_gc = socket_check_gc, + .may_gc = socket_may_gc, .sigchld_event = socket_sigchld_event, diff --git a/src/core/socket.h b/src/core/socket.h index 9c528fb39c..84ec9cff08 100644 --- a/src/core/socket.h +++ b/src/core/socket.h @@ -104,8 +104,8 @@ struct Socket { DynamicCreds dynamic_creds; /* For Accept=no sockets refers to the one service we'll - activate. For Accept=yes sockets is either NULL, or filled - when the next service we spawn. */ + * activate. For Accept=yes sockets is either NULL, or filled + * to refer to the next service we spawn. */ UnitRef service; SocketState state, deserialized_state; diff --git a/src/core/swap.c b/src/core/swap.c index fde0c24089..ed397c4c9e 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -978,12 +978,15 @@ _pure_ static const char *swap_sub_state_to_string(Unit *u) { return swap_state_to_string(SWAP(u)->state); } -_pure_ static bool swap_check_gc(Unit *u) { +_pure_ static bool swap_may_gc(Unit *u) { Swap *s = SWAP(u); assert(s); - return s->from_proc_swaps; + if (s->from_proc_swaps) + return false; + + return true; } static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) { @@ -1505,7 +1508,7 @@ const UnitVTable swap_vtable = { .active_state = swap_active_state, .sub_state_to_string = swap_sub_state_to_string, - .check_gc = swap_check_gc, + .may_gc = swap_may_gc, .sigchld_event = swap_sigchld_event, diff --git a/src/core/unit.c b/src/core/unit.c index bd077cb359..ceeb76c696 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -336,20 +336,25 @@ int unit_set_description(Unit *u, const char *description) { return 0; } -bool unit_check_gc(Unit *u) { +bool unit_may_gc(Unit *u) { UnitActiveState state; int r; assert(u); - /* Checks whether the unit is ready to be unloaded for garbage collection. Returns true, when the unit shall - * stay around, false if there's no reason to keep it loaded. */ + /* Checks whether the unit is ready to be unloaded for garbage collection. + * Returns true when the unit may be collected, and false if there's some + * reason to keep it loaded. + * + * References from other units are *not* checked here. Instead, this is done + * in unit_gc_sweep(), but using markers to properly collect dependency loops. + */ if (u->job) - return true; + return false; if (u->nop_job) - return true; + return false; state = unit_active_state(u); @@ -359,26 +364,23 @@ bool unit_check_gc(Unit *u) { UNIT_VTABLE(u)->release_resources(u); if (u->perpetual) - return true; - - if (u->refs) - return true; + return false; if (sd_bus_track_count(u->bus_track) > 0) - return true; + return false; /* But we keep the unit object around for longer when it is referenced or configured to not be gc'ed */ switch (u->collect_mode) { case COLLECT_INACTIVE: if (state != UNIT_INACTIVE) - return true; + return false; break; case COLLECT_INACTIVE_OR_FAILED: if (!IN_SET(state, UNIT_INACTIVE, UNIT_FAILED)) - return true; + return false; break; @@ -394,14 +396,13 @@ bool unit_check_gc(Unit *u) { if (r < 0) log_unit_debug_errno(u, r, "Failed to determine whether cgroup %s is empty: %m", u->cgroup_path); if (r <= 0) - return true; + return false; } - if (UNIT_VTABLE(u)->check_gc) - if (UNIT_VTABLE(u)->check_gc(u)) - return true; + if (UNIT_VTABLE(u)->may_gc && !UNIT_VTABLE(u)->may_gc(u)) + return false; - return false; + return true; } void unit_add_to_load_queue(Unit *u) { @@ -431,7 +432,7 @@ void unit_add_to_gc_queue(Unit *u) { if (u->in_gc_queue || u->in_cleanup_queue) return; - if (unit_check_gc(u)) + if (!unit_may_gc(u)) return; LIST_PREPEND(gc_queue, u->manager->gc_unit_queue, u); @@ -609,27 +610,6 @@ void unit_free(Unit *u) { for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++) bidi_set_free(u, u->dependencies[d]); - if (u->type != _UNIT_TYPE_INVALID) - LIST_REMOVE(units_by_type, u->manager->units_by_type[u->type], u); - - if (u->in_load_queue) - LIST_REMOVE(load_queue, u->manager->load_queue, u); - - if (u->in_dbus_queue) - LIST_REMOVE(dbus_queue, u->manager->dbus_unit_queue, u); - - if (u->in_cleanup_queue) - LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u); - - if (u->in_gc_queue) - LIST_REMOVE(gc_queue, u->manager->gc_unit_queue, u); - - if (u->in_cgroup_realize_queue) - LIST_REMOVE(cgroup_realize_queue, u->manager->cgroup_realize_queue, u); - - if (u->in_cgroup_empty_queue) - LIST_REMOVE(cgroup_empty_queue, u->manager->cgroup_empty_queue, u); - if (u->on_console) manager_unref_console(u->manager); @@ -643,28 +623,32 @@ void unit_free(Unit *u) { (void) manager_update_failed_units(u->manager, u, false); set_remove(u->manager->startup_units, u); - free(u->description); - strv_free(u->documentation); - free(u->fragment_path); - free(u->source_path); - strv_free(u->dropin_paths); - free(u->instance); - - free(u->job_timeout_reboot_arg); - - set_free_free(u->names); - unit_unwatch_all_pids(u); - condition_free_list(u->conditions); - condition_free_list(u->asserts); - - free(u->reboot_arg); - unit_ref_unset(&u->slice); + while (u->refs_by_target) + unit_ref_unset(u->refs_by_target); - while (u->refs) - unit_ref_unset(u->refs); + if (u->type != _UNIT_TYPE_INVALID) + LIST_REMOVE(units_by_type, u->manager->units_by_type[u->type], u); + + if (u->in_load_queue) + LIST_REMOVE(load_queue, u->manager->load_queue, u); + + if (u->in_dbus_queue) + LIST_REMOVE(dbus_queue, u->manager->dbus_unit_queue, u); + + if (u->in_gc_queue) + LIST_REMOVE(gc_queue, u->manager->gc_unit_queue, u); + + if (u->in_cgroup_realize_queue) + LIST_REMOVE(cgroup_realize_queue, u->manager->cgroup_realize_queue, u); + + if (u->in_cgroup_empty_queue) + LIST_REMOVE(cgroup_empty_queue, u->manager->cgroup_empty_queue, u); + + if (u->in_cleanup_queue) + LIST_REMOVE(cleanup_queue, u->manager->cleanup_queue, u); safe_close(u->ip_accounting_ingress_map_fd); safe_close(u->ip_accounting_egress_map_fd); @@ -677,6 +661,22 @@ void unit_free(Unit *u) { bpf_program_unref(u->ip_bpf_ingress); bpf_program_unref(u->ip_bpf_egress); + condition_free_list(u->conditions); + condition_free_list(u->asserts); + + free(u->description); + strv_free(u->documentation); + free(u->fragment_path); + free(u->source_path); + strv_free(u->dropin_paths); + free(u->instance); + + free(u->job_timeout_reboot_arg); + + set_free_free(u->names); + + free(u->reboot_arg); + free(u); } @@ -896,8 +896,8 @@ int unit_merge(Unit *u, Unit *other) { return r; /* Redirect all references */ - while (other->refs) - unit_ref_set(other->refs, u); + while (other->refs_by_target) + unit_ref_set(other->refs_by_target, other->refs_by_target->source, u); /* Merge dependencies */ for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++) @@ -1119,7 +1119,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { "%s\tActive Enter Timestamp: %s\n" "%s\tActive Exit Timestamp: %s\n" "%s\tInactive Enter Timestamp: %s\n" - "%s\tGC Check Good: %s\n" + "%s\tMay GC: %s\n" "%s\tNeed Daemon Reload: %s\n" "%s\tTransient: %s\n" "%s\tPerpetual: %s\n" @@ -1137,7 +1137,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { prefix, strna(format_timestamp(timestamp2, sizeof(timestamp2), u->active_enter_timestamp.realtime)), prefix, strna(format_timestamp(timestamp3, sizeof(timestamp3), u->active_exit_timestamp.realtime)), prefix, strna(format_timestamp(timestamp4, sizeof(timestamp4), u->inactive_enter_timestamp.realtime)), - prefix, yes_no(unit_check_gc(u)), + prefix, yes_no(unit_may_gc(u)), prefix, yes_no(unit_need_daemon_reload(u)), prefix, yes_no(u->transient), prefix, yes_no(u->perpetual), @@ -2964,8 +2964,7 @@ int unit_set_slice(Unit *u, Unit *slice) { if (UNIT_ISSET(u->slice) && u->cgroup_realized) return -EBUSY; - unit_ref_unset(&u->slice); - unit_ref_set(&u->slice, slice); + unit_ref_set(&u->slice, u, slice); return 1; } @@ -3974,30 +3973,32 @@ int unit_get_unit_file_preset(Unit *u) { return u->unit_file_preset; } -Unit* unit_ref_set(UnitRef *ref, Unit *u) { +Unit* unit_ref_set(UnitRef *ref, Unit *source, Unit *target) { assert(ref); - assert(u); + assert(source); + assert(target); - if (ref->unit) + if (ref->target) unit_ref_unset(ref); - ref->unit = u; - LIST_PREPEND(refs, u->refs, ref); - return u; + ref->source = source; + ref->target = target; + LIST_PREPEND(refs_by_target, target->refs_by_target, ref); + return target; } void unit_ref_unset(UnitRef *ref) { assert(ref); - if (!ref->unit) + if (!ref->target) return; /* We are about to drop a reference to the unit, make sure the garbage collection has a look at it as it might * be unreferenced now. */ - unit_add_to_gc_queue(ref->unit); + unit_add_to_gc_queue(ref->target); - LIST_REMOVE(refs, ref->unit->refs, ref); - ref->unit = NULL; + LIST_REMOVE(refs_by_target, ref->target->refs_by_target, ref); + ref->source = ref->target = NULL; } static int user_from_unit_name(Unit *u, char **ret) { diff --git a/src/core/unit.h b/src/core/unit.h index 617f044e24..5cab2211bb 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -123,8 +123,8 @@ struct UnitRef { * that we can merge two units if necessary and correct all * references to them */ - Unit* unit; - LIST_FIELDS(UnitRef, refs); + Unit *source, *target; + LIST_FIELDS(UnitRef, refs_by_target); }; typedef enum UnitCGroupBPFState { @@ -187,7 +187,7 @@ struct Unit { char *job_timeout_reboot_arg; /* References to this */ - LIST_HEAD(UnitRef, refs); + LIST_HEAD(UnitRef, refs_by_target); /* Conditions to check */ LIST_HEAD(Condition, conditions); @@ -490,10 +490,9 @@ struct UnitVTable { /* Additionally to UnitActiveState determine whether unit is to be restarted. */ bool (*will_restart)(Unit *u); - /* Return true when there is reason to keep this entry around - * even nothing references it and it isn't active in any - * way */ - bool (*check_gc)(Unit *u); + /* Return false when there is a reason to prevent this unit from being gc'ed + * even though nothing references it and it isn't active in any way. */ + bool (*may_gc)(Unit *u); /* When the unit is not running and no job for it queued we shall release its runtime resources */ void (*release_resources)(Unit *u); @@ -626,7 +625,7 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c); int unit_choose_id(Unit *u, const char *name); int unit_set_description(Unit *u, const char *description); -bool unit_check_gc(Unit *u); +bool unit_may_gc(Unit *u); void unit_add_to_load_queue(Unit *u); void unit_add_to_dbus_queue(Unit *u); @@ -728,11 +727,11 @@ void unit_trigger_notify(Unit *u); UnitFileState unit_get_unit_file_state(Unit *u); int unit_get_unit_file_preset(Unit *u); -Unit* unit_ref_set(UnitRef *ref, Unit *u); +Unit* unit_ref_set(UnitRef *ref, Unit *source, Unit *target); void unit_ref_unset(UnitRef *ref); -#define UNIT_DEREF(ref) ((ref).unit) -#define UNIT_ISSET(ref) (!!(ref).unit) +#define UNIT_DEREF(ref) ((ref).target) +#define UNIT_ISSET(ref) (!!(ref).target) int unit_patch_contexts(Unit *u); diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index b99e7abf91..46e7c06ddc 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -639,16 +639,16 @@ int seat_get_idle_hint(Seat *s, dual_timestamp *t) { return idle_hint; } -bool seat_check_gc(Seat *s, bool drop_not_started) { +bool seat_may_gc(Seat *s, bool drop_not_started) { assert(s); if (drop_not_started && !s->started) - return false; - - if (seat_is_seat0(s)) return true; - return seat_has_master_device(s); + if (seat_is_seat0(s)) + return false; + + return !seat_has_master_device(s); } void seat_add_to_gc_queue(Seat *s) { diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h index 5427ac21c8..4982ea3381 100644 --- a/src/login/logind-seat.h +++ b/src/login/logind-seat.h @@ -79,7 +79,7 @@ int seat_start(Seat *s); int seat_stop(Seat *s, bool force); int seat_stop_sessions(Seat *s, bool force); -bool seat_check_gc(Seat *s, bool drop_not_started); +bool seat_may_gc(Seat *s, bool drop_not_started); void seat_add_to_gc_queue(Seat *s); bool seat_name_is_valid(const char *name); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 92eb2943fe..1859150b5e 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1000,27 +1000,27 @@ static void session_remove_fifo(Session *s) { } } -bool session_check_gc(Session *s, bool drop_not_started) { +bool session_may_gc(Session *s, bool drop_not_started) { assert(s); if (drop_not_started && !s->started) - return false; + return true; if (!s->user) - return false; + return true; if (s->fifo_fd >= 0) { if (pipe_eof(s->fifo_fd) <= 0) - return true; + return false; } if (s->scope_job && manager_job_is_active(s->manager, s->scope_job)) - return true; + return false; if (s->scope && manager_unit_is_active(s->manager, s->scope)) - return true; + return false; - return false; + return true; } void session_add_to_gc_queue(Session *s) { diff --git a/src/login/logind-session.h b/src/login/logind-session.h index 8491832402..16a278c792 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -131,7 +131,7 @@ struct Session { Session *session_new(Manager *m, const char *id); void session_free(Session *s); void session_set_user(Session *s, User *u); -bool session_check_gc(Session *s, bool drop_not_started); +bool session_may_gc(Session *s, bool drop_not_started); void session_add_to_gc_queue(Session *s); int session_activate(Session *s); bool session_is_active(Session *s); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 32b2045696..f85564f221 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -680,25 +680,25 @@ int user_check_linger_file(User *u) { return access(p, F_OK) >= 0; } -bool user_check_gc(User *u, bool drop_not_started) { +bool user_may_gc(User *u, bool drop_not_started) { assert(u); if (drop_not_started && !u->started) - return false; + return true; if (u->sessions) - return true; + return false; if (user_check_linger_file(u) > 0) - return true; + return false; if (u->slice_job && manager_job_is_active(u->manager, u->slice_job)) - return true; + return false; if (u->service_job && manager_job_is_active(u->manager, u->service_job)) - return true; + return false; - return false; + return true; } void user_add_to_gc_queue(User *u) { diff --git a/src/login/logind-user.h b/src/login/logind-user.h index ad1686ffc5..c3452dcd08 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -66,7 +66,7 @@ User *user_free(User *u); DEFINE_TRIVIAL_CLEANUP_FUNC(User *, user_free); -bool user_check_gc(User *u, bool drop_not_started); +bool user_may_gc(User *u, bool drop_not_started); void user_add_to_gc_queue(User *u); int user_start(User *u); int user_stop(User *u, bool force); diff --git a/src/login/logind.c b/src/login/logind.c index d9b5f026cf..3a6ab7fcb6 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -958,7 +958,7 @@ static void manager_gc(Manager *m, bool drop_not_started) { LIST_REMOVE(gc_queue, m->seat_gc_queue, seat); seat->in_gc_queue = false; - if (!seat_check_gc(seat, drop_not_started)) { + if (seat_may_gc(seat, drop_not_started)) { seat_stop(seat, false); seat_free(seat); } @@ -969,14 +969,14 @@ static void manager_gc(Manager *m, bool drop_not_started) { session->in_gc_queue = false; /* First, if we are not closing yet, initiate stopping */ - if (!session_check_gc(session, drop_not_started) && + if (session_may_gc(session, drop_not_started) && session_get_state(session) != SESSION_CLOSING) session_stop(session, false); /* Normally, this should make the session referenced * again, if it doesn't then let's get rid of it * immediately */ - if (!session_check_gc(session, drop_not_started)) { + if (session_may_gc(session, drop_not_started)) { session_finalize(session); session_free(session); } @@ -987,11 +987,11 @@ static void manager_gc(Manager *m, bool drop_not_started) { user->in_gc_queue = false; /* First step: queue stop jobs */ - if (!user_check_gc(user, drop_not_started)) + if (user_may_gc(user, drop_not_started)) user_stop(user, false); /* Second step: finalize user */ - if (!user_check_gc(user, drop_not_started)) { + if (user_may_gc(user, drop_not_started)) { user_finalize(user); user_free(user); } diff --git a/src/machine/machine.c b/src/machine/machine.c index 7375d83a44..4bacf91d26 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -486,22 +486,22 @@ int machine_finalize(Machine *m) { return 0; } -bool machine_check_gc(Machine *m, bool drop_not_started) { +bool machine_may_gc(Machine *m, bool drop_not_started) { assert(m); if (m->class == MACHINE_HOST) - return true; - - if (drop_not_started && !m->started) return false; - if (m->scope_job && manager_job_is_active(m->manager, m->scope_job)) + if (drop_not_started && !m->started) return true; + if (m->scope_job && manager_job_is_active(m->manager, m->scope_job)) + return false; + if (m->unit && manager_unit_is_active(m->manager, m->unit)) - return true; + return false; - return false; + return true; } void machine_add_to_gc_queue(Machine *m) { diff --git a/src/machine/machine.h b/src/machine/machine.h index 1ee82ffe81..1ef5dcdb89 100644 --- a/src/machine/machine.h +++ b/src/machine/machine.h @@ -85,7 +85,7 @@ struct Machine { Machine* machine_new(Manager *manager, MachineClass class, const char *name); void machine_free(Machine *m); -bool machine_check_gc(Machine *m, bool drop_not_started); +bool machine_may_gc(Machine *m, bool drop_not_started); void machine_add_to_gc_queue(Machine *m); int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error); int machine_stop(Machine *m); diff --git a/src/machine/machined.c b/src/machine/machined.c index 34b2024043..9fb67882e1 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -292,14 +292,14 @@ void manager_gc(Manager *m, bool drop_not_started) { machine->in_gc_queue = false; /* First, if we are not closing yet, initiate stopping */ - if (!machine_check_gc(machine, drop_not_started) && + if (machine_may_gc(machine, drop_not_started) && machine_get_state(machine) != MACHINE_CLOSING) machine_stop(machine); /* Now, the stop probably made this referenced * again, but if it didn't, then it's time to let it * go entirely. */ - if (!machine_check_gc(machine, drop_not_started)) { + if (machine_may_gc(machine, drop_not_started)) { machine_finalize(machine); machine_free(machine); } diff --git a/src/test/test-strv.c b/src/test/test-strv.c index 76cd551eeb..c6698f4396 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -676,6 +676,17 @@ static void test_strv_make_nulstr(void) { test_strv_make_nulstr_one(STRV_MAKE("foo", "bar", "quuux")); } +static void test_strv_free_free(void) { + char ***t; + + assert_se(t = new(char**, 3)); + assert_se(t[0] = strv_new("a", "b", NULL)); + assert_se(t[1] = strv_new("c", "d", "e", NULL)); + t[2] = NULL; + + t = strv_free_free(t); +} + static void test_foreach_string(void) { const char * const t[] = { "foo", @@ -763,6 +774,7 @@ int main(int argc, char *argv[]) { test_strv_skip(); test_strv_extend_n(); test_strv_make_nulstr(); + test_strv_free_free(); test_foreach_string(); test_strv_fnmatch();