[systemd-devel] Conflation of propagation in dependencies creates race windows

[Jonathon Kowalski]

On Sat, Jan 19, 2019 at 5:05 PM Uoti Urpala <uoti.urpala at pp1.inet.fi> wrote:
>
> On Sat, 2019-01-19 at 15:54 +0000, Jonathon Kowalski wrote:
> > https://github.com/systemd/systemd/issues/1154 which is similar in
> > nature convinces me that systemd currently conflates two many
> > properties in the same dependency. The second bug in particular would
> > not happen if there was a version of Requires= that disabled the
> > PartOf= stuff it currently has, i.e., pick and choose deps.
>
> I think you're wrong here. It makes perfect sense that if unit A has
> Requires= for another unit, stopping that required unit which A can't
> work without will stop A too. Removing that logic is not a good
> solution.
>

I am NOT advocating for changing how Requires= currently works, and
also, no many people by accident use Requires= for its semantics at
startup.

In particular, you could argue that stopping X.service should also
stop Y.service, and maybe I'd agree, but there is certainly a need for
RequiresThatOnlyHasStartupEffect=.

For example, I take a oneshot unit. I have Requires=oneshot.service
from my service. systemd through job processing will ensure that if
oneshot fails (instead of going to inactive), my unit's start job
ordered After=oneshot.service is invalidated too. However, using
Requires= to get such semantics now gives me PartOf= for free too:

* If I ever run systemctl stop oneshot.service, my service gets a stop
job forwarded to it due to RequiredBy=
* If I ever run systemctl restart oneshot.service, my service gets a
stop job forwarded to it due to RequiredBy=

In both cases, the oneshot is inactive. No, taking into consideration
the unit state is not the solution here when generating transaction,
as that is TOCTTOU in nature.

> So the case is:
>
> Service X has StopWhenUnneeded=true
> Service Y has Requires=X, Restart=always
>
> and the problem is that Y dying can in some circumstances stop X (due
> to it being "unneeded" when Y is not actively running), and then
> running this stop action on X stops Y completely too (so it will not
> restart later), as if the administrator had explicitly stopped X.
>
> I think the ideal behavior here is that X would never be stopped at all
> if Y is scheduled to be restarted. Changes that would keep Y running
> even if the administrator explicitly runs "systemctl stop X" would
> definitely be wrong.
>
>

There is no internal distinction between systemtl stop X and stop job
triggered due to propagation, so I am not sure if that will be
feasible. Also, I am not asking for Requires= to be changed, I am
asking for something Requires= does at startup to be exposed
separately, then you can combine it with PartOf= to get the same
semantics (and I already give another convincing use case above).

Y is also only scheduled to be "restarted" only after it reaches
inactive/failed (depending on policy, timer will be armed). I don't
see why systemd should change that. Then, if you systemctl stop
X.service, it would still restart, following your solution, because
reaching that state would not be attributed to job. ExecStopPost=
creates a time window where an external job comes in and installs
itself, and then the state change is attributed to it. The unexpected
bool internally remains false and service never enters restart phase.

Also, this cannot work. Suppose I have Restart=on-failure in service,
and service exits on its own normally. How will systemd decide X
should not be stopped, in case an ExecStop* statement ends up failing,
and then it *should* restart our service? All of this is going to be
very racy and undeterministic.

> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel