From 791cd1599311d2cfffec9cc7d2915b36cbd3047d Mon Sep 17 00:00:00 2001 From: Jonathon Kowalski Date: Sat, 19 Jan 2019 05:19:46 +0000 Subject: [PATCH] Fail RequisiteOf units with oneshots Fixes: #11422 Oneshots going to inactive directly without ever entering UNIT_ACTIVE is considered success. This however means that if something both Requires= and Requisites= a unit of such nature, the verify-active job getting merged into the start job makes it lose this property of failing the depending jobs, as there, the start job has the result JOB_DONE on success, so we never walk over RequisiteOf units. This change makes sure that such units always go down. It is also only meaningful with After=, but so is Requisite= itself. Also, we also catch cases like a oneshot having RemainAfterExit= true making us start up properly in such a setting, but then removing it, reloading the unit, and restarting it. In such a case, we go down due to restart propagation before them, and our start job waits on theirs, properly failing with the JOB_DEPENDENCY result. This covers cases where ConditionXYZ= creates a similar situation as well. --- src/core/job.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/core/job.c b/src/core/job.c index 59bb9d2162..b2aa0c600f 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1027,6 +1027,31 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool alr job_fail_dependencies(u, UNIT_CONFLICTED_BY); } + /* A special check to make sure we take down anything RequisiteOf if we + * aren't active. This is when the verify-active job merges with a + * satisfying job type, and then loses it's invalidation effect, as the + * result there is JOB_DONE for the start job we merged into, while we + * should be failing the depending job if the said unit isn't infact + * active. Oneshots are an example of this, where going directly from + * activating to inactive is success. + * + * This happens when you use ConditionXYZ= in a unit too, since in that + * case the job completes with the JOB_DONE result, but the unit never + * really becomes active. Note that such a case still involves merging: + * + * A start job waits for something else, and a verify-active comes in + * and merges in the installed job. Then, later, when it becomes + * runnable, it finishes with JOB_DONE result as execution on conditions + * not being met is skipped, breaking our dependency semantics. + * + * Also, depending on if start job waits or not, the merging may or may + * not happen (the verify-active job may trigger after it finishes), so + * you get undeterministic results without this check. + */ + if (result == JOB_DONE && recursive && !UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u))) { + if (IN_SET(t, JOB_START, JOB_RELOAD)) + job_fail_dependencies(u, UNIT_REQUISITE_OF); + } /* Trigger OnFailure dependencies that are not generated by * the unit itself. We don't treat JOB_CANCELED as failure in * this context. And JOB_FAILURE is already handled by the