morgan/systemd
1
0
Fork 0
mirror of https://github.com/morgan9e/systemd synced 2026-04-14 08:25:20 +09:00
Code Issues Packages Projects Releases Wiki Activity

core/transaction: restart dependency loop when a new dependency is added (#37465)

Browse Source 
Fixes #36031.
This commit is contained in:
Yu Watanabe
2025-05-31 04:01:06 +09:00
committed by GitHub
parent e018cf0c67 7824e70a07
commit ab5587d674
4 changed files with 135 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
src/core/transaction.c
View File

@@ -880,7 +880,7 @@ void transaction_add_propagate_reload_jobs(
assert(tr);
assert(unit);
UNIT_FOREACH_DEPENDENCY(dep, unit, UNIT_ATOM_PROPAGATES_RELOAD_TO) {
UNIT_FOREACH_DEPENDENCY_SAFE(dep, unit, UNIT_ATOM_PROPAGATES_RELOAD_TO) {
_cleanup_(sd_bus_error_free) sd_bus_error e = SD_BUS_ERROR_NULL;
nt = job_type_collapse(JOB_TRY_RELOAD, dep);
@@ -935,7 +935,7 @@ int transaction_add_job_and_dependencies(
sd_bus_error *e) {
bool is_new;
Job *ret;
Job *job;
int r;
assert(tr);
@@ -944,8 +944,8 @@ int transaction_add_job_and_dependencies(
assert(type < _JOB_TYPE_MAX_IN_TRANSACTION);
assert(unit);
/* Before adding jobs for this unit, let's ensure that its state has been loaded This matters when
* jobs are spawned as part of coldplugging itself (see e. g. path_coldplug()). This way, we
/* Before adding jobs for this unit, let's ensure that its state has been loaded. This matters when
* jobs are spawned as part of coldplugging itself (see e. g. path_coldplug()). This way, we
* "recursively" coldplug units, ensuring that we do not look at state of not-yet-coldplugged
* units. */
if (MANAGER_IS_RELOADING(unit->manager))
@@ -960,7 +960,6 @@ int transaction_add_job_and_dependencies(
return sd_bus_error_setf(e, BUS_ERROR_LOAD_FAILED, "Unit %s is not loaded properly.", unit->id);
if (type != JOB_STOP) {
r = bus_unit_validate_load_state(unit, e);
/* The time-based cache allows new units to be started without daemon-reload, but if they are
* already referenced (because of dependencies or ordering) then we have to force a load of
* the fragment. As an optimization, check first if anything in the usual paths was modified
@@ -971,14 +970,15 @@ int transaction_add_job_and_dependencies(
*
* Given building up the transaction is a synchronous operation, attempt
* to load the unit immediately. */
if (r < 0 && manager_unit_cache_should_retry_load(unit)) {
sd_bus_error_free(e);
if (manager_unit_cache_should_retry_load(unit)) {
assert(unit->load_state == UNIT_NOT_FOUND);
unit->load_state = UNIT_STUB;
r = unit_load(unit);
if (r < 0 || unit->load_state == UNIT_STUB)
unit->load_state = UNIT_NOT_FOUND;
r = bus_unit_validate_load_state(unit, e);
unit->load_error = 0;
(void) unit_load(unit);
assert(unit->load_state != UNIT_STUB);
}
r = bus_unit_validate_load_state(unit, e);
if (r < 0)
return r;
}
@@ -999,24 +999,24 @@ int transaction_add_job_and_dependencies(
}
/* First add the job. */
ret = transaction_add_one_job(tr, type, unit, &is_new);
if (!ret)
job = transaction_add_one_job(tr, type, unit, &is_new);
if (!job)
return -ENOMEM;
if (FLAGS_SET(flags, TRANSACTION_IGNORE_ORDER))
ret->ignore_order = true;
job->ignore_order = true;
/* Then, add a link to the job. */
if (by) {
if (!job_dependency_new(by, ret, FLAGS_SET(flags, TRANSACTION_MATTERS), FLAGS_SET(flags, TRANSACTION_CONFLICTS)))
if (!job_dependency_new(by, job, FLAGS_SET(flags, TRANSACTION_MATTERS), FLAGS_SET(flags, TRANSACTION_CONFLICTS)))
return -ENOMEM;
} else {
/* If the job has no parent job, it is the anchor job. */
assert(!tr->anchor_job);
tr->anchor_job = ret;
tr->anchor_job = job;
if (FLAGS_SET(flags, TRANSACTION_REENQUEUE_ANCHOR))
ret->refuse_late_merge = true;
job->refuse_late_merge = true;
}
if (!is_new || FLAGS_SET(flags, TRANSACTION_IGNORE_REQUIREMENTS) || type == JOB_NOP)
@@ -1026,9 +1026,9 @@ int transaction_add_job_and_dependencies(
Unit *dep;
/* If we are following some other unit, make sure we add all dependencies of everybody following. */
if (unit_following_set(ret->unit, &following) > 0)
if (unit_following_set(job->unit, &following) > 0)
SET_FOREACH(dep, following) {
r = transaction_add_job_and_dependencies(tr, type, dep, ret, flags & TRANSACTION_IGNORE_ORDER, e);
r = transaction_add_job_and_dependencies(tr, type, dep, job, flags & TRANSACTION_IGNORE_ORDER, e);
if (r < 0) {
log_unit_full_errno(dep, r == -ERFKILL ? LOG_INFO : LOG_WARNING, r,
"Cannot add dependency job, ignoring: %s",
@@ -1039,8 +1039,8 @@ int transaction_add_job_and_dependencies(
/* Finally, recursively add in all dependencies. */
if (IN_SET(type, JOB_START, JOB_RESTART)) {
UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PULL_IN_START) {
r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER), e);
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PULL_IN_START) {
r = transaction_add_job_and_dependencies(tr, JOB_START, dep, job, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER), e);
if (r < 0) {
if (r != -EBADR) /* job type not applicable */
goto fail;
@@ -1049,8 +1049,8 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PULL_IN_START_IGNORED) {
r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, flags & TRANSACTION_IGNORE_ORDER, e);
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PULL_IN_START_IGNORED) {
r = transaction_add_job_and_dependencies(tr, JOB_START, dep, job, flags & TRANSACTION_IGNORE_ORDER, e);
if (r < 0) {
/* unit masked, job type not applicable and unit not found are not considered
* as errors. */
@@ -1062,8 +1062,8 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PULL_IN_VERIFY) {
r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER), e);
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PULL_IN_VERIFY) {
r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, job, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER), e);
if (r < 0) {
if (r != -EBADR) /* job type not applicable */
goto fail;
@@ -1072,8 +1072,8 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PULL_IN_STOP) {
r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, TRANSACTION_MATTERS | TRANSACTION_CONFLICTS | (flags & TRANSACTION_IGNORE_ORDER), e);
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PULL_IN_STOP) {
r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, job, TRANSACTION_MATTERS | TRANSACTION_CONFLICTS | (flags & TRANSACTION_IGNORE_ORDER), e);
if (r < 0) {
if (r != -EBADR) /* job type not applicable */
goto fail;
@@ -1082,8 +1082,8 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PULL_IN_STOP_IGNORED) {
r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, flags & TRANSACTION_IGNORE_ORDER, e);
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PULL_IN_STOP_IGNORED) {
r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, job, flags & TRANSACTION_IGNORE_ORDER, e);
if (r < 0) {
log_unit_warning(dep,
"Cannot add dependency job, ignoring: %s",
@@ -1096,7 +1096,7 @@ int transaction_add_job_and_dependencies(
if (IN_SET(type, JOB_RESTART, JOB_STOP) || (type == JOB_START && FLAGS_SET(flags, TRANSACTION_PROPAGATE_START_AS_RESTART))) {
bool is_stop = type == JOB_STOP;
UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PROPAGATE_STOP) {
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PROPAGATE_STOP) {
/* We propagate RESTART only as TRY_RESTART, in order not to start dependencies that
* are not around. */
JobType nt;
@@ -1105,7 +1105,7 @@ int transaction_add_job_and_dependencies(
if (nt == JOB_NOP)
continue;
r = transaction_add_job_and_dependencies(tr, nt, dep, ret, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER), e);
r = transaction_add_job_and_dependencies(tr, nt, dep, job, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER), e);
if (r < 0) {
if (r != -EBADR) /* job type not applicable */
return r;
@@ -1118,7 +1118,7 @@ int transaction_add_job_and_dependencies(
* all other dependencies are processed, i.e. we're the anchor job or already in the recursion
* that handles it. */
if (!by || FLAGS_SET(flags, TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL))
UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PROPAGATE_STOP_GRACEFUL) {
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PROPAGATE_STOP_GRACEFUL) {
JobType nt;
Job *j;
@@ -1128,7 +1128,7 @@ int transaction_add_job_and_dependencies(
if (nt == JOB_NOP)
continue;
r = transaction_add_job_and_dependencies(tr, nt, dep, ret, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER) | TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL, e);
r = transaction_add_job_and_dependencies(tr, nt, dep, job, TRANSACTION_MATTERS | (flags & TRANSACTION_IGNORE_ORDER) | TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL, e);
if (r < 0) {
if (r != -EBADR) /* job type not applicable */
return r;
@@ -1139,7 +1139,7 @@ int transaction_add_job_and_dependencies(
}
if (type == JOB_RELOAD)
transaction_add_propagate_reload_jobs(tr, ret->unit, ret, flags & TRANSACTION_IGNORE_ORDER);
transaction_add_propagate_reload_jobs(tr, job->unit, job, flags & TRANSACTION_IGNORE_ORDER);
/* JOB_VERIFY_ACTIVE requires no dependency handling */
@@ -1150,7 +1150,7 @@ fail:
log_unit_debug_errno(unit, r, "Cannot add dependency job to transaction, deleting job %s/%s again: %s",
unit->id, job_type_to_string(type), bus_error_message(e, r));
transaction_delete_job(tr, ret, /* delete_dependencies= */ false);
transaction_delete_job(tr, job, /* delete_dependencies= */ false);
return r;
}
@@ -1216,7 +1216,7 @@ int transaction_add_triggering_jobs(Transaction *tr, Unit *u) {
assert(tr);
assert(u);
UNIT_FOREACH_DEPENDENCY(trigger, u, UNIT_ATOM_TRIGGERED_BY) {
UNIT_FOREACH_DEPENDENCY_SAFE(trigger, u, UNIT_ATOM_TRIGGERED_BY) {
_cleanup_(sd_bus_error_free) sd_bus_error e = SD_BUS_ERROR_NULL;
/* No need to stop inactive jobs */

10
src/core/unit.c
View File

@@ -631,12 +631,14 @@ static void unit_clear_dependencies(Unit *u) {
hashmap_remove(other_deps, u);
unit_add_to_gc_queue(other);
other->dependency_generation++;
}
hashmap_free(deps);
}
u->dependencies = hashmap_free(u->dependencies);
u->dependency_generation++;
}
static void unit_remove_transient(Unit *u) {
@@ -1094,6 +1096,9 @@ static void unit_merge_dependencies(Unit *u, Unit *other) {
}
other->dependencies = hashmap_free(other->dependencies);
u->dependency_generation++;
other->dependency_generation++;
}
int unit_merge(Unit *u, Unit *other) {
@@ -3088,6 +3093,7 @@ static int unit_add_dependency_impl(
return r;
flags = NOTIFY_DEPENDENCY_UPDATE_FROM;
u->dependency_generation++;
}
if (other_info.data != other_info_old.data) {
@@ -3104,6 +3110,7 @@ static int unit_add_dependency_impl(
}
flags |= NOTIFY_DEPENDENCY_UPDATE_TO;
other->dependency_generation++;
}
return flags;
@@ -5618,6 +5625,9 @@ void unit_remove_dependencies(Unit *u, UnitDependencyMask mask) {
/* The unit 'other' may not be wanted by the unit 'u'. */
unit_submit_to_stop_when_unneeded_queue(other);
u->dependency_generation++;
other->dependency_generation++;
done = false;
break;
}

31
src/core/unit.h
View File

@@ -238,6 +238,7 @@ typedef struct Unit {
* and whose value encodes why the dependency exists, using the UnitDependencyInfo type. i.e. a
* Hashmap(UnitDependency → Hashmap(Unit* → UnitDependencyInfo)) */
Hashmap *dependencies;
uint64_t dependency_generation;
/* Similar, for RequiresMountsFor= and WantsMountsFor= path dependencies. The key is the path, the
* value the UnitDependencyInfo type */
@@ -1152,27 +1153,44 @@ CollectMode collect_mode_from_string(const char *s) _pure_;
typedef struct UnitForEachDependencyData {
/* Stores state for the FOREACH macro below for iterating through all deps that have any of the
* specified dependency atom bits set */
const Unit *unit;
UnitDependencyAtom match_atom;
Hashmap *by_type, *by_unit;
void *current_type;
Iterator by_type_iterator, by_unit_iterator;
Unit **current_unit;
uint64_t generation;
unsigned n_restart;
bool restart_on_generation_change;
} UnitForEachDependencyData;
/* Let's not restart the loop infinitely. */
#define MAX_FOREACH_DEPENDENCY_RESTART 100000
/* Iterates through all dependencies that have a specific atom in the dependency type set. This tries to be
* smart: if the atom is unique, we'll directly go to right entry. Otherwise we'll iterate through the
* per-dependency type hashmap and match all dep that have the right atom set. */
#define _UNIT_FOREACH_DEPENDENCY(other, u, ma, data) \
#define _UNIT_FOREACH_DEPENDENCY(other, u, ma, restart, data) \
for (UnitForEachDependencyData data = { \
.unit = (u), \
.match_atom = (ma), \
.by_type = (u)->dependencies, \
.by_type_iterator = ITERATOR_FIRST, \
.current_unit = &(other), \
.restart_on_generation_change = (restart), \
}; \
({ \
UnitDependency _dt = _UNIT_DEPENDENCY_INVALID; \
bool _found; \
\
if (data.generation == 0 || \
(data.restart_on_generation_change && \
data.generation != data.unit->dependency_generation)) { \
data.generation = data.unit->dependency_generation; \
data.by_type = data.unit->dependencies; \
data.by_type_iterator = ITERATOR_FIRST; \
assert_se(data.n_restart++ < MAX_FOREACH_DEPENDENCY_RESTART); \
} else \
assert(data.generation == data.unit->dependency_generation); \
\
if (data.by_type && ITERATOR_IS_FIRST(data.by_type_iterator)) { \
_dt = unit_dependency_from_unique_atom(data.match_atom); \
if (_dt >= 0) { \
@@ -1185,12 +1203,13 @@ typedef struct UnitForEachDependencyData {
if (_dt < 0) \
_found = hashmap_iterate(data.by_type, \
&data.by_type_iterator, \
(void**)&(data.by_unit), \
(void**) &(data.by_unit), \
(const void**) &(data.current_type)); \
_found; \
}); ) \
if ((unit_dependency_to_atom(UNIT_DEPENDENCY_FROM_PTR(data.current_type)) & data.match_atom) != 0) \
for (data.by_unit_iterator = ITERATOR_FIRST; \
data.generation == data.unit->dependency_generation && \
hashmap_iterate(data.by_unit, \
&data.by_unit_iterator, \
NULL, \
@@ -1198,7 +1217,9 @@ typedef struct UnitForEachDependencyData {
/* Note: this matches deps that have *any* of the atoms specified in match_atom set */
#define UNIT_FOREACH_DEPENDENCY(other, u, match_atom) \
_UNIT_FOREACH_DEPENDENCY(other, u, match_atom, UNIQ_T(data, UNIQ))
_UNIT_FOREACH_DEPENDENCY(other, u, match_atom, false, UNIQ_T(data, UNIQ))
#define UNIT_FOREACH_DEPENDENCY_SAFE(other, u, match_atom) \
_UNIT_FOREACH_DEPENDENCY(other, u, match_atom, true, UNIQ_T(data, UNIQ))
#define _LOG_CONTEXT_PUSH_UNIT(unit, u, c) \
const Unit *u = (unit); \

64
test/units/TEST-23-UNIT-FILE.start-stop-no-reload.sh
View File

@@ -10,7 +10,14 @@ set -o pipefail
at_exit() {
set +e
rm -f /run/systemd/system/TEST-23-UNIT-FILE-no-reload.{service,target}
rm -f /run/systemd/system/TEST-23-UNIT-FILE-no-reload.target
rm -f /run/systemd/system/TEST-23-UNIT-FILE-no-reload.service
rm -f /run/systemd/system/TEST-23-UNIT-FILE-no-reload-2.service
rm -f /run/systemd/system/TEST-23-UNIT-FILE-no-reload-3.service
systemctl stop TEST-23-UNIT-FILE-no-reload.target
systemctl stop TEST-23-UNIT-FILE-no-reload.service
systemctl stop TEST-23-UNIT-FILE-no-reload-2.service
systemctl stop TEST-23-UNIT-FILE-no-reload-3.service
}
trap at_exit EXIT
@@ -91,3 +98,58 @@ EOF
systemctl restart TEST-23-UNIT-FILE-no-reload.target
systemctl is-active TEST-23-UNIT-FILE-no-reload.service
# Stop and remove, and try again to exercise https://github.com/systemd/systemd/issues/36031
systemctl stop TEST-23-UNIT-FILE-no-reload.service TEST-23-UNIT-FILE-no-reload.target
rm -f /run/systemd/system/TEST-23-UNIT-FILE-no-reload.service /run/systemd/system/TEST-23-UNIT-FILE-no-reload.target
systemctl daemon-reload
sleep 3.1
cat >/run/systemd/system/TEST-23-UNIT-FILE-no-reload.target <<EOF
[Unit]
Conflicts=shutdown.target
EOF
cat >/run/systemd/system/TEST-23-UNIT-FILE-no-reload.service <<EOF
[Unit]
Conflicts=TEST-23-UNIT-FILE-no-reload.target
Wants=TEST-23-UNIT-FILE-no-reload-2.service
Wants=TEST-23-UNIT-FILE-no-reload-3.service
[Service]
ExecStart=sleep infinity
EOF
systemctl daemon-reload
systemctl start TEST-23-UNIT-FILE-no-reload.service
(! systemctl is-active TEST-23-UNIT-FILE-no-reload.target )
systemctl is-active TEST-23-UNIT-FILE-no-reload.service
[[ "$(systemctl show --property LoadState --value TEST-23-UNIT-FILE-no-reload-2.service)" == not-found ]]
[[ "$(systemctl show --property LoadState --value TEST-23-UNIT-FILE-no-reload-3.service)" == not-found ]]
cat >/run/systemd/system/TEST-23-UNIT-FILE-no-reload-2.service <<EOF
[Unit]
Conflicts=TEST-23-UNIT-FILE-no-reload.target
[Service]
ExecStart=sleep infinity
EOF
# This service file is intentionally invalid (Type=exec without ExecStart=).
cat >/run/systemd/system/TEST-23-UNIT-FILE-no-reload-3.service <<EOF
[Unit]
Conflicts=TEST-23-UNIT-FILE-no-reload.target
[Service]
Type=exec
ExecStop=sleep infinity
EOF
systemctl start TEST-23-UNIT-FILE-no-reload.target
systemctl is-active TEST-23-UNIT-FILE-no-reload.target
(! systemctl is-active TEST-23-UNIT-FILE-no-reload.service )
(! systemctl is-active TEST-23-UNIT-FILE-no-reload-2.service )
(! systemctl is-active TEST-23-UNIT-FILE-no-reload-3.service )
[[ "$(systemctl show --property LoadState --value TEST-23-UNIT-FILE-no-reload-2.service)" == loaded ]]
[[ "$(systemctl show --property LoadState --value TEST-23-UNIT-FILE-no-reload-3.service)" == bad-setting ]]
[[ "$(systemctl show --property Conflicts --value TEST-23-UNIT-FILE-no-reload-2.service)" =~ TEST-23-UNIT-FILE-no-reload.target ]]
[[ "$(systemctl show --property Conflicts --value TEST-23-UNIT-FILE-no-reload-3.service)" =~ TEST-23-UNIT-FILE-no-reload.target ]]