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

core: introduce Unit.dependency_generation counter and restart loop when dependency is updated in the loop

Browse Source 
When starting unit A, a dependent unit B may be loaded if it is not
loaded yet, and the dependencies in unit A may be updated.
As Hashmap does not allow a new entry to be added in a loop, we need to
restart loop in such case.

Fixes a bug introduced by cda667722c.
Fixes #36031.
This commit is contained in:
Yu Watanabe
2025-05-15 12:34:35 +09:00
parent 9b6aa9e443
commit b7777d0884
3 changed files with 45 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
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);
@@ -1039,7 +1039,7 @@ int transaction_add_job_and_dependencies(
/* Finally, recursively add in all dependencies. */
if (IN_SET(type, JOB_START, JOB_RESTART)) {
UNIT_FOREACH_DEPENDENCY(dep, job->unit, UNIT_ATOM_PULL_IN_START) {
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 */
@@ -1049,7 +1049,7 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, job->unit, UNIT_ATOM_PULL_IN_START_IGNORED) {
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
@@ -1062,7 +1062,7 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, job->unit, UNIT_ATOM_PULL_IN_VERIFY) {
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 */
@@ -1072,7 +1072,7 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, job->unit, UNIT_ATOM_PULL_IN_STOP) {
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 */
@@ -1082,7 +1082,7 @@ int transaction_add_job_and_dependencies(
}
}
UNIT_FOREACH_DEPENDENCY(dep, job->unit, UNIT_ATOM_PULL_IN_STOP_IGNORED) {
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,
@@ -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, job->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;
@@ -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, job->unit, UNIT_ATOM_PROPAGATE_STOP_GRACEFUL) {
UNIT_FOREACH_DEPENDENCY_SAFE(dep, job->unit, UNIT_ATOM_PROPAGATE_STOP_GRACEFUL) {
JobType nt;
Job *j;
@@ -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); \