Bug#736258: systemd invoke-rc.d stop unsafe if socket is activated

[Henrique de Moraes Holschuh]

On Sat, 26 Jul 2014, Michael Biebl wrote:
> Am 26.07.2014 22:15, schrieb Henrique de Moraes Holschuh:
> >> Up until now, we do not generate explicit maintainer scripts code to
> >> stop the socket during upgrades. The underlying idea was, that keeping
> >> the socket open would allow to not lose any incoming message.
> > 
> > This is, unfortunately, exactly the wrong thing to do in many cases.
> 
> Let me add here, that there are very few packages currently shipping
> socket activated services.
> And we back then concluded that it should be up to the package
> maintainer to decide what kind of behaviour he wants for his package
> during upgrades.

IMO this is a Bad Idea.  It fails the least-surprise rule, it has already
been misused, and it will continue to introduce subtle bugs because people
WILL misuse it.

It was easy to notice with acpid only because its users will reconnect on
socket close() and the upgrade process "objected" to it in a very
noticeable, if harmless way.

Next time, it might not be that easy to debug, or that harmless.

I can easily see this "feature" causing multi-component services to run[1]
with a mix of old and new components loaded because the activation came in
at a bad time.  It is a rare service indeed that would test for this, expect
this, or support this upstream.

[1] preferably during a really short time window, but that depends on
postinst doing a forced service restart instead of just trying to start the
service.

> If invoke-rc.d by default stops both .socket and .service, the package
> maintainer no longer has this option.

This is incorrect.

You can extend "invoke-rc.d stop" with a --option that "quiesces the service
and terminates its running instances, but allows a (possibly immediate)
on-demand restart of the service by triggers such as socket activation"

Only packages that explicitly ask for the new behaviour would get it.
Backwards compatibility is preserved.

Where on-demand triggers are unsupported, it would just fallback to the
regular "invoke-rc.d stop" behavior, so it can be unconditionally used by
packages that can deal with it.

Evidently, this mode requires that the package maintainer scripts *restart*
the service at appropriate points.  "invoke-rc.d start" won't cut it.

> There is no "systemd's invoke-rc.d". The only existing invoke-rc.d
> implementation is shipped in sysv-rc.

Once you guys have a solution, please file/reassign a bug there with the
desired patch, requesting a severity=emergency upload.

> > Changing that would be *really* painful, and require that we audit a very
> > large number of packages. 
> 
> Well, not really. Atm only very few packages have socket activation
> enabled and this is an explicit action by the package maintainer. We

ATM.  In the long run, it would have dropped for good the (implicit?
expected?) promise of "invoke-rc.d stop" that the service would remain
disabled until either the local admin or a future call to invoke-rc.d
restarted it.

If we change the invoke-rc.d API in a non-backwards-compatible way,
everthing that uses it would have to be audited.  We wouldn't have to do it
right away as the only thing that could trigger it right now is socket
activation, but we would still have to do it eventually.

> If a package uses restart-after-upgrade (and has no further system
> scripts) there actually isn't any problem during upgrades, only the
> removal case needs an explicit stop of the socket.

Unless it is a multi-module service (like say, postfix), or it objects to
the new format of some of its static configuration files, and so on.

It is safe in a lot of cases (prossibly even in the more common ones), but
you really cannot expect it to be safe in the general case.

> That said, package maintainers atm need to be aware of this for now and
> deal with it accordingly, i.e. by explicitly managing the socket in
> their maintainer scripts (and accompanying system scripts).

Please let's just fix this to do the obvious thing for the standard case
(stop the socket), which happens to be the backwards-compatible solution.

The new functionality can be added as an --option to "invoke-rc.d stop".

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh