From ce359e98f87c7fbe5aa1d275e43cf088ce46c2b0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 3 Feb 2016 23:38:50 +0100 Subject: [PATCH] core: when a service's ExecStartPre= times out, skip ExecStop= This makes sure we never run two control processes at the same time, we cannot keep track off. This introduces a slight change of behaviour but cleans up the definition of ExecStop= and ExecStopPost=. The former is now invoked only if the service managed to start-up correctly. The latter is called even if start-up failed half-way. Thus, ExecStopPost= may be used as clean-up step for both successful and failed start-up attempts, but ExecStop='s purpose is clearly defined as being responsible for shutting down the service and nothing else. The precise behaviour of this was not documented yet. This commit adds the necessary docs. Fixes: #1254 --- man/systemd.service.xml | 42 +++++++++++++++++++++++++++++++----------- src/core/service.c | 4 ++-- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 14f6cd6..4cd36ac 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -383,6 +383,11 @@ used to start long-running processes. All processes forked off by processes invoked via ExecStartPre= will be killed before the next service process is run. + + Note that if any of the commands specified in ExecStartPre=, + ExecStart=, or ExecStartPost= fail (and are not prefixed with + -, see above) or time out before the service is fully up, execution continues with commands + specified in ExecStopPost=, the commands in ExecStop= are skipped. @@ -438,21 +443,36 @@ SIGKILL immediately after the command exited, this would not result in a clean stop. The specified command should hence be a synchronous operation, not an - asynchronous one. + asynchronous one. + + Note that the commands specified in ExecStop= are only executed when the service + started successfuly first. They are not invoked if the service was never started at all, or in case its + start-up failed, for example because any of the commands specified in ExecStart=, + ExecStartPre= or ExecStartPost= failed (and weren't prefixed with + -, see above) or timed out. Use ExecStopPost= to invoke commands when a + service failed to start up correctly and is shut down again. + + It is recommended to use this setting for commands that communicate with the service requesting clean + termination. When the commands specified with this option are executed it should be assumed that the service is + still fully up and is able to react correctly to all commands. For post-mortem clean-up steps use + ExecStopPost= instead. ExecStopPost= - Additional commands that are executed after - the service was stopped. This includes cases where the - commands configured in ExecStop= were used, - where the service does not have any - ExecStop= defined, or where the service - exited unexpectedly. This argument takes multiple command - lines, following the same scheme as described for - ExecStart=. Use of these settings is - optional. Specifier and environment variable substitution is - supported. + Additional commands that are executed after the service is stopped. This includes cases where + the commands configured in ExecStop= were used, where the service does not have any + ExecStop= defined, or where the service exited unexpectedly. This argument takes multiple + command lines, following the same scheme as described for ExecStart=. Use of these settings + is optional. Specifier and environment variable substitution is supported. Note that – unlike + ExecStop= – commands specified with this setting are invoked when a service failed to start + up correctly and is shut down again. + + It is recommended to use this setting for clean-up operations that shall be executed even when the + service failed to start up correctly. Commands configured with this setting need to be able to operate even if + the service failed starting up half-way and left incompletely initialized data around. As the service's + processes have been terminated already when the commands specified with this setting are executed they should + not attempt to communicate with them. diff --git a/src/core/service.c b/src/core/service.c index a9345e3..4942fc4 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2788,7 +2788,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { case SERVICE_START_POST: if (f != SERVICE_SUCCESS) { - service_enter_stop(s, f); + service_enter_signal(s, SERVICE_STOP_SIGTERM, f); break; } @@ -2878,7 +2878,7 @@ static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *us case SERVICE_START_POST: log_unit_warning(UNIT(s), "Start-post operation timed out. Stopping."); - service_enter_stop(s, SERVICE_FAILURE_TIMEOUT); + service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_TIMEOUT); break; case SERVICE_RUNNING: -- 2.7.4