Skip to content

Commit a911bb9

Browse files
poetteringpoettering
committed
core: watch SIGCHLD more closely to track processes of units with no reliable cgroup empty notifier
When a process dies that we can associate with a specific unit, start watching all other processes of that unit, so that we can associate those processes with the unit too. Also, for service units start doing this as soon as we get the first SIGCHLD for either control or main process, so that we can follow the processes of the service from one to the other, as long as process that remain are processes of the ones we watched that died and got reassigned to us as parent. Similar, for scope units start doing this as soon as the scope controller abandons the unit, and thus management entirely reverts to systemd. To abandon a unit introduce a new Abandon() scope unit method call.
1 parent 1006a62 commit a911bb9

File tree

7 files changed

+281
-108
lines changed

7 files changed

+281
-108
lines changed

‎src/core/dbus-scope.c‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,16 @@
2828
#include "bus-util.h"
2929
#include "bus-internal.h"
3030

31+
static int bus_scope_abandon(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
32+
Scope *s = userdata;
33+
34+
assert(bus);
35+
assert(message);
36+
assert(s);
37+
38+
return scope_abandon(s);
39+
}
40+
3141
static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, scope_result, ScopeResult);
3242

3343
const sd_bus_vtable bus_scope_vtable[] = {
@@ -36,6 +46,7 @@ const sd_bus_vtable bus_scope_vtable[] = {
3646
SD_BUS_PROPERTY("TimeoutStopUSec", "t", bus_property_get_usec, offsetof(Scope, timeout_stop_usec), SD_BUS_VTABLE_PROPERTY_CONST),
3747
SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Scope, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
3848
SD_BUS_SIGNAL("RequestStop", NULL, 0),
49+
SD_BUS_METHOD("Abandon", NULL, NULL, bus_scope_abandon, 0),
3950
SD_BUS_VTABLE_END
4051
};
4152

@@ -56,10 +67,6 @@ static int bus_scope_set_transient_property(
5667
unsigned n = 0;
5768
uint32_t pid;
5869

59-
r = set_ensure_allocated(&s->pids, trivial_hash_func, trivial_compare_func);
60-
if (r < 0)
61-
return r;
62-
6370
r = sd_bus_message_enter_container(message, 'a', "u");
6471
if (r < 0)
6572
return r;
@@ -70,7 +77,7 @@ static int bus_scope_set_transient_property(
7077
return -EINVAL;
7178

7279
if (mode != UNIT_CHECK) {
73-
r = set_put(s->pids, LONG_TO_PTR(pid));
80+
r = unit_watch_pid(UNIT(s), pid);
7481
if (r < 0 && r != -EEXIST)
7582
return r;
7683
}

‎src/core/manager.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1459,7 +1459,7 @@ static int manager_dispatch_sigchld(Manager *m) {
14591459
log_debug_unit(u->id,
14601460
"Child %lu belongs to %s", (long unsigned) si.si_pid, u->id);
14611461

1462-
hashmap_remove(m->watch_pids, LONG_TO_PTR(si.si_pid));
1462+
unit_unwatch_pid(u, si.si_pid);
14631463
UNIT_VTABLE(u)->sigchld_event(u, si.si_pid, si.si_code, si.si_status);
14641464
}
14651465

‎src/core/scope.c‎

Lines changed: 66 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
static const UnitActiveState state_translation_table[_SCOPE_STATE_MAX] = {
3636
[SCOPE_DEAD] = UNIT_INACTIVE,
3737
[SCOPE_RUNNING] = UNIT_ACTIVE,
38+
[SCOPE_ABANDONED] = UNIT_ACTIVE,
3839
[SCOPE_STOP_SIGTERM] = UNIT_DEACTIVATING,
3940
[SCOPE_STOP_SIGKILL] = UNIT_DEACTIVATING,
4041
[SCOPE_FAILED] = UNIT_FAILED
@@ -66,9 +67,6 @@ static void scope_done(Unit *u) {
6667

6768
free(s->controller);
6869

69-
set_free(s->pids);
70-
s->pids = NULL;
71-
7270
s->timer_event_source = sd_event_source_unref(s->timer_event_source);
7371
}
7472

@@ -100,15 +98,14 @@ static void scope_set_state(Scope *s, ScopeState state) {
10098
old_state = s->state;
10199
s->state = state;
102100

103-
if (state != SCOPE_STOP_SIGTERM &&
104-
state != SCOPE_STOP_SIGKILL)
101+
if (!IN_SET(state, SCOPE_STOP_SIGTERM, SCOPE_STOP_SIGKILL))
105102
s->timer_event_source = sd_event_source_unref(s->timer_event_source);
106103

104+
if (IN_SET(state, SCOPE_DEAD, SCOPE_FAILED))
105+
unit_unwatch_all_pids(UNIT(s));
106+
107107
if (state != old_state)
108-
log_debug("%s changed %s -> %s",
109-
UNIT(s)->id,
110-
scope_state_to_string(old_state),
111-
scope_state_to_string(state));
108+
log_debug("%s changed %s -> %s", UNIT(s)->id, scope_state_to_string(old_state), scope_state_to_string(state));
112109

113110
unit_notify(UNIT(s), state_translation_table[old_state], state_translation_table[state], true);
114111
}
@@ -135,7 +132,7 @@ static int scope_verify(Scope *s) {
135132
if (UNIT(s)->load_state != UNIT_LOADED)
136133
return 0;
137134

138-
if (set_size(s->pids) <= 0 && UNIT(s)->manager->n_reloading <= 0) {
135+
if (set_isempty(UNIT(s)->pids) && UNIT(s)->manager->n_reloading <= 0) {
139136
log_error_unit(UNIT(s)->id, "Scope %s has no PIDs. Refusing.", UNIT(s)->id);
140137
return -EINVAL;
141138
}
@@ -181,12 +178,15 @@ static int scope_coldplug(Unit *u) {
181178

182179
if (s->deserialized_state != s->state) {
183180

184-
if (s->deserialized_state == SCOPE_STOP_SIGKILL || s->deserialized_state == SCOPE_STOP_SIGTERM) {
181+
if (IN_SET(s->deserialized_state, SCOPE_STOP_SIGKILL, SCOPE_STOP_SIGTERM)) {
185182
r = scope_arm_timer(s);
186183
if (r < 0)
187184
return r;
188185
}
189186

187+
if (!IN_SET(s->deserialized_state, SCOPE_DEAD, SCOPE_FAILED))
188+
unit_watch_all_pids(UNIT(s));
189+
190190
scope_set_state(s, s->deserialized_state);
191191
}
192192

@@ -227,6 +227,8 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) {
227227
if (f != SCOPE_SUCCESS)
228228
s->result = f;
229229

230+
unit_watch_all_pids(UNIT(s));
231+
230232
/* If we have a controller set let's ask the controller nicely
231233
* to terminate the scope, instead of us going directly into
232234
* SIGTERM beserk mode */
@@ -288,13 +290,10 @@ static int scope_start(Unit *u) {
288290
return r;
289291
}
290292

291-
r = cg_attach_many_everywhere(u->manager->cgroup_supported, u->cgroup_path, s->pids);
293+
r = cg_attach_many_everywhere(u->manager->cgroup_supported, u->cgroup_path, UNIT(s)->pids);
292294
if (r < 0)
293295
return r;
294296

295-
set_free(s->pids);
296-
s->pids = NULL;
297-
298297
s->result = SCOPE_SUCCESS;
299298

300299
scope_set_state(s, SCOPE_RUNNING);
@@ -305,13 +304,13 @@ static int scope_stop(Unit *u) {
305304
Scope *s = SCOPE(u);
306305

307306
assert(s);
308-
assert(s->state == SCOPE_RUNNING);
309307

310308
if (s->state == SCOPE_STOP_SIGTERM ||
311309
s->state == SCOPE_STOP_SIGKILL)
312310
return 0;
313311

314-
assert(s->state == SCOPE_RUNNING);
312+
assert(s->state == SCOPE_RUNNING ||
313+
s->state == SCOPE_ABANDONED);
315314

316315
scope_enter_signal(s, SCOPE_STOP_SIGTERM, SCOPE_SUCCESS);
317316
return 0;
@@ -389,15 +388,41 @@ static bool scope_check_gc(Unit *u) {
389388
/* Never clean up scopes that still have a process around,
390389
* even if the scope is formally dead. */
391390

392-
if (UNIT(s)->cgroup_path) {
393-
r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, UNIT(s)->cgroup_path, true);
391+
if (u->cgroup_path) {
392+
r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, true);
394393
if (r <= 0)
395394
return true;
396395
}
397396

398397
return false;
399398
}
400399

400+
static void scope_notify_cgroup_empty_event(Unit *u) {
401+
Scope *s = SCOPE(u);
402+
assert(u);
403+
404+
log_debug_unit(u->id, "%s: cgroup is empty", u->id);
405+
406+
if (IN_SET(s->state, SCOPE_RUNNING, SCOPE_ABANDONED, SCOPE_STOP_SIGTERM, SCOPE_STOP_SIGKILL))
407+
scope_enter_dead(s, SCOPE_SUCCESS);
408+
}
409+
410+
static void scope_sigchld_event(Unit *u, pid_t pid, int code, int status) {
411+
412+
/* If we get a SIGCHLD event for one of the processes we were
413+
interested in, then we look for others to watch, under the
414+
assumption that we'll sooner or later get a SIGCHLD for
415+
them, as the original process we watched was probably the
416+
parent of them, and they are hence now our children. */
417+
418+
unit_tidy_watch_pids(u, 0, 0);
419+
unit_watch_all_pids(u);
420+
421+
/* If the PID set is empty now, then let's finish this off */
422+
if (set_isempty(u->pids))
423+
scope_notify_cgroup_empty_event(u);
424+
}
425+
401426
static int scope_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata) {
402427
Scope *s = SCOPE(userdata);
403428

@@ -429,24 +454,30 @@ static int scope_dispatch_timer(sd_event_source *source, usec_t usec, void *user
429454
return 0;
430455
}
431456

432-
static void scope_notify_cgroup_empty_event(Unit *u) {
433-
Scope *s = SCOPE(u);
434-
assert(u);
457+
int scope_abandon(Scope *s) {
458+
assert(s);
435459

436-
log_debug_unit(u->id, "%s: cgroup is empty", u->id);
460+
if (!IN_SET(s->state, SCOPE_RUNNING, SCOPE_ABANDONED))
461+
return -ESTALE;
437462

438-
switch (s->state) {
463+
free(s->controller);
464+
s->controller = NULL;
439465

440-
case SCOPE_RUNNING:
441-
case SCOPE_STOP_SIGTERM:
442-
case SCOPE_STOP_SIGKILL:
443-
scope_enter_dead(s, SCOPE_SUCCESS);
466+
/* The client is no longer watching the remaining processes,
467+
* so let's step in here, under the assumption that the
468+
* remaining processes will be sooner or later reassigned to
469+
* us as parent. */
444470

445-
break;
471+
unit_tidy_watch_pids(UNIT(s), 0, 0);
472+
unit_watch_all_pids(UNIT(s));
446473

447-
default:
448-
;
449-
}
474+
/* If the PID set is empty now, then let's finish this off */
475+
if (set_isempty(UNIT(s)->pids))
476+
scope_notify_cgroup_empty_event(UNIT(s));
477+
else
478+
scope_set_state(s, SCOPE_ABANDONED);
479+
480+
return 0;
450481
}
451482

452483
_pure_ static UnitActiveState scope_active_state(Unit *u) {
@@ -464,6 +495,7 @@ _pure_ static const char *scope_sub_state_to_string(Unit *u) {
464495
static const char* const scope_state_table[_SCOPE_STATE_MAX] = {
465496
[SCOPE_DEAD] = "dead",
466497
[SCOPE_RUNNING] = "running",
498+
[SCOPE_ABANDONED] = "abandoned",
467499
[SCOPE_STOP_SIGTERM] = "stop-sigterm",
468500
[SCOPE_STOP_SIGKILL] = "stop-sigkill",
469501
[SCOPE_FAILED] = "failed",
@@ -516,6 +548,8 @@ const UnitVTable scope_vtable = {
516548

517549
.check_gc = scope_check_gc,
518550

551+
.sigchld_event = scope_sigchld_event,
552+
519553
.reset_failed = scope_reset_failed,
520554

521555
.notify_cgroup_empty = scope_notify_cgroup_empty_event,

‎src/core/scope.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ typedef struct Scope Scope;
2929
typedef enum ScopeState {
3030
SCOPE_DEAD,
3131
SCOPE_RUNNING,
32+
SCOPE_ABANDONED,
3233
SCOPE_STOP_SIGTERM,
3334
SCOPE_STOP_SIGKILL,
3435
SCOPE_FAILED,
@@ -57,13 +58,13 @@ struct Scope {
5758

5859
char *controller;
5960

60-
Set *pids;
61-
6261
sd_event_source *timer_event_source;
6362
};
6463

6564
extern const UnitVTable scope_vtable;
6665

66+
int scope_abandon(Scope *s);
67+
6768
const char* scope_state_to_string(ScopeState i) _const_;
6869
ScopeState scope_state_from_string(const char *s) _pure_;
6970

0 commit comments

Comments
 (0)