system/systemd
system/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd synced 2024-10-14 20:17:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

core/service: rework management of exec_fd event source

Browse source 
The code in service_spawn() was written as if exec_fd_event_source
was always unset. (We would either fail the assertion that is moved in the
patch, or leak the event source object if it was set.)

To make this work, let's always assert that exec_fd_event_source is unset,
and actually unset it service_sigchld_event(). I think this is the most
elegant approach. The problem is that we don't have the same information
about execution flags as in service_spawn(), so we need to conditionalize
on pid==main_pid to know if we should disable exec_fd_event_source.
I think this matches all cases where we may set exec_fd_event_source:
service_enter_start() and service_run_next_main().

service_enter_stop_post() calls service_set_state(), which will also destroy
the source. But that happens too late, because from service_enter_stop_post()
we call service_spawn() first, and then service_set_state() second.

(An alternative approach would be to deallocate the existing
exec_fd_event_source in service_spawn(). But this would mean that we would
temporarily have an event source attached to a process that we already know is
dead, which seems less than ideal.)

Original report from Dimitri John Ledkov <dimitri.ledkov@canonical.com>:
> Ubuntu private bug reference for this issue at the moment is
> https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1921145

> Michael's and Ian's team run into an issue when using systemd in the
> initrd, without dbus daemon running, and launching a unit in a
> particular way that appears to lock up systemd (pid 1) it self.

> michael vogt: "The attached script works for me to reproduce this on
> classic. I tested 20.04 (245) and 21.04 (247) in a qemu VM. Sometimes
> I need to run it multiple times but usually it crashes after at most 2
> runs. Use "journalctl | tail" to see the messages, it's the same that
> Ian reported. There is also a /var/crash/_usr_lib_systemd_systemd
> crash file created."

> I understand that the particular way to run a unit is very odd,
> however, it is currently possible to invoke, and it would be expected
> for pid1 to not lock up and crash.

> The Assertion that systemd hits is along the lines of:

> [ 10.182627] systemd[1]: Assertion 's' failed at
> src/core/service.c:3204, function service_dispatch_exec_io().
> Aborting.
> [ 10.195458] systemd[1]: Caught <ABRT>, dumped core as pid 449.
> [ 10.204446] systemd[1]: Freezing execution.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2021-05-10 13:12:53 +02:00
parent 199475092d
commit bc989831e6
1 changed files with 8 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
src/core/service.c
View file

@ -1351,7 +1351,7 @@ static int service_allocate_exec_fd_event_source(
if (r < 0)
return log_unit_error_errno(UNIT(s), r, "Failed to adjust priority of exec_fd event source: %m");
(void) sd_event_source_set_description(source, "service event_fd");
(void) sd_event_source_set_description(source, "service exec_fd");
r = sd_event_source_set_io_fd_own(source, true);
if (r < 0)
@ -1433,6 +1433,8 @@ static int service_spawn(
if (r < 0)
return r;
assert(!s->exec_fd_event_source);
if (flags & EXEC_IS_CONTROL) {
/* If this is a control process, mask the permissions/chroot application if this is requested. */
if (s->permissions_start_only)
@ -1458,8 +1460,6 @@ static int service_spawn(
}
if (!FLAGS_SET(flags, EXEC_IS_CONTROL) && s->type == SERVICE_EXEC) {
assert(!s->exec_fd_event_source);
r = service_allocate_exec_fd(s, &exec_fd_source, &exec_params.exec_fd);
if (r < 0)
return r;
@ -3391,6 +3391,11 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
else
clean_mode = EXIT_CLEAN_DAEMON;
if (s->main_pid == pid)
/* Clean up the exec_fd event source. The source owns its end of the pipe, so this will close
* that too. */
s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source);
if (is_clean_exit(code, status, clean_mode, &s->success_status))
f = SERVICE_SUCCESS;
else if (code == CLD_EXITED)