Users can mistakenly run init.d scripts and cause problems if an equivalent upstart job already exists

Thank you for taking the time to report this bug and helping to make Ubuntu better.

Confirmed. I expect /etc/init.d/mysql to continue to be a symlink to /lib/init/upstart-job, but this does not appear to be the case. Instead, /etc/init.d/mysql is the init script from Debian packaging.

However, I don't see a symlink in /etc/rc2.d, so I believe that the upstart job is the only job used. Therefore, the issue occurs only if the /etc/init.d/mysql script is used manually. The "service" command appears to work correctly with upstart.

Workaround: don't use the /etc/init.d/ script directly.

Not affected: mysql-server-5.5 5.5.34-0ubuntu0.13.04.1 on Raring
Affected: mysql-server-5.5 5.5.35-0ubuntu0.13.10.1 on Saucy
Affected: mysql-server-5.5 5.5.35-0ubuntu1 on Trusty

I just spoke to Stéphane Graber about this. AIUI, this should be fixed in lsb-base, so that Sys V init scripts that source /lib/lsb/init-functions will correctly handle the case that an upstart job exists and not let you run it.

lsb -> upstart, because upstart will just provide lsb integration snippet

Please take a look at https://bugs.launchpad.net/ubuntu/+source/dbus/+bug/1072518 and let us know if this is the same issue or not.

This bug was fixed in the package upstart - 1.12.1-0ubuntu6

---------------
upstart (1.12.1-0ubuntu6) utopic; urgency=medium

  * Map force-reload to restart command in init-functions hook.
  * Use variable expansion, instead of basename as that is shipped in
    /usr.
  * Resolve test-failure, when dbus user session job is launched
    accidentally by unit-tests. (LP: #1315767)
 -- Dimitri John Ledkov <email address hidden> Wed, 07 May 2014 20:15:35 +0100

upstart init-functions.d hook has ended up being shipped by lsb, since circular dependencies onto upstart from sysv are a bad idea. Hence re-assigning SRU to lsb.

If a "stop" on the mysql init script does not work, the init script should be removed.

Hi, any progress/update for this ?

Hi Dimtri,
   Why it can be fixed for utopic but not trusty, they both still use upstart by default.. right ?

@zhhuabj read my comment https://bugs.launchpad.net/ubuntu/+source/mysql-5.5/+bug/1273462/comments/6 I have correct the status and the location of where the fix needs to be backported to, without changing the effective status. In utopic the bug was fixed in upstart, but in trusty it needs to be fixed in lsb package as that's where the file to be patched should be at. Nobody did the work yet to cherrypick this fix into trusty, and it's open for anybody to be picked up.

@Edward,
      I tested above trusty.debdiff, but I found the constraint "Replaces: upstart (<< 1.12.1-0ubuntu8) && Breaks: upstart (<< 1.12.1-0ubuntu8)" prevents installing lsb-base package. error logs pls refer the link [1], so I remove them. I will upload trusty_v2.debdiff.
     Then I built a local deb binary lsb package and test, it looks good to me, the test result pls refer the link [2].

[1], https://pastebin.canonical.com/135504/
[2], https://pastebin.canonical.com/135505/

Hi Hua,
Thanks for the patch, I can confirm that this patch fixed the issues I had with MySQL.

Updated patch with bug number and version changed.

Updated patch with bug number and version changed.

I'd like to see more thought in the Regression Potential section, because it clearly is NOT 'None'.

Please search though and check if any existing packages call /etc/init.d/<service name> either through any maintscripts, or if any other scripts will call /etc/init.d/<service name>. For each of these ensure that a 'No-op' is the correct response, otherwise you'll need to patch each of these packages to do the correct thing.

For example 'freeradius' calls '/etc/init.d/freeradius reload' in its logrotate script, (bug 1406105) this would no-op that function (which maybe is better than what it does), but makes me think we need to very carefully check if any other packages do this kind of thing too.

Chris,
   Could you review [Regression Potential] section I updated just now ?
   I think this should be the responsibility of existing packages to make they are doing the correct thing. This change in lsb should be in the right road, and the same change has also landed into Utopic, so this change should also land in Trusty. maybe what we can do now is try to enumerate this kinds of bad packages. right ? I also have some search for the call /etc/init.d/<service name>, but I did find at the moment except freeradius you mentioned in bug 1406105. Do you have any better way to deal with this tricky situation ? thanks.

With my SRU/TB hat on, I'd prefer making individual changes to e. g. mysql and other packages rather than changing this wholesale. Even if we did review all packages in Ubuntu which ship an init script that still leaves third-party scripts (and there are a lot of them). So I strongly recommend not to change the LSB package itself.

I think the change description within the patch is wrong (or at least, misleading/incomplete). What this patch actually does is:
 - for anything that calls /etc/init.d/foo, if /etc/init/foo.conf exists, make the call a passthrough to upstart instead of calling the init script.
 - for anything that calls /etc/rc?.d/[KS]??foo, make it a no-op.

The *only* thing that's made a no-op is the invocation of the init script *by the rc?.d symlink*. This is something that should never happen anyway; our dependency-based boot is already supposed to bypass invoking this script in favor of the upstart job. Any system that has been configured to not use the dependency-based boot in trusty is going to have race conditions at boot because of the combination of upstart job and init script, and while making this change may change the characteristics of the behavior at boot it would not be a regression because things are already broken.

If anything interfaces with the /etc/init.d/foo script, that will still function, except that now it will *work* by passing through to the upstart job instead of trying to manipulate processes via (possibly non-existent) pid files etc

> I think this should be the responsibility of existing packages to make they are doing the correct thing.

That is not sufficient for an SRU. Your SRU needs to be compatible with the existing packages in the archive /as they are/, not as you declare they should be.

However, per my previous comment, I don't think this is actually a problem.

Steve Langasek [2015-09-01 21:38 -0000]:
> The *only* thing that's made a no-op is the invocation of the init
> script *by the rc?.d symlink*. This is something that should never
> happen anyway; our dependency-based boot is already supposed to bypass
> invoking this script in favor of the upstart job. Any system that has
> been configured to not use the dependency-based boot in trusty is going
> to have race conditions at boot because of the combination of upstart
> job and init script

I suppose you are talking about insserv here. Note that trusty did
*not* use that yet, it was enabled in utopic
(https://launchpad.net/ubuntu/+source/sysvinit/2.88dsf-41ubuntu13) and
it took a lot of fine-tuning and fixing to mop up the fallout.

Thus in trusty you don't have the option to use dependency-based
sysvinit scripts, you *have* to have rc?.d symlinks; and they are
being used through upstart too via /etc/init/rc.conf.

So this change does influence the boot significantly, and I'm
not yet convinced that it necessarily regression proof? Or am I still
missing something?

> Thus in trusty you don't have the option to use dependency-based
> sysvinit scripts, you *have* to have rc?.d symlinks; and they are
> being used through upstart too via /etc/init/rc.conf.

In that case, any /etc/rc?.d symlink that shadows an upstart job is still going to be doing the wrong thing at boot by trying to start a service directly instead of via upstart.

I don't think anything here is going to be a "regression". Only "changes in undefined and racy behavior". Yes, users may see some surprising changes in boot-time behavior as a result of this change; those will hopefully be changes for the better, but anything that relies on the current behavior of running both the init script and the upstart job at boot is deeply buggy and not supportable and should not block this SRU.

I still disagree that it's "undefined behaviour" -- if we have used the actual init.d script so far to start e. g. mysql under upstart, then this does not sound either racy nor undefined; it surely isn't what we *intended* to do, though?

My concern is, this fix will change behaviour for *all* packages shipping upstart jobs and sysvinit scripts, and it is not a given that the two do equivalent things. So if we now call /etc/init/frobnicator.conf instead of /etc/init.d/frobnicator and the former has a bug or a changed behaviour, that would be a regression?

On Wed, Sep 02, 2015 at 03:26:49PM -0000, Martin Pitt wrote:
> I still disagree that it's "undefined behaviour" -- if we have used the
> actual init.d script so far to start e. g. mysql under upstart,

That is *not* what we're doing. Installing the mysql-server-5.5 package on
trusty gives you:

$ ls -l /etc/rc?.d/*mysql* /etc/init.d/*mysql* /etc/init/*mysql*
ls: cannot access /etc/rc?.d/*mysql*: No such file or directory
-rwxr-xr-x 1 root root 5491 Feb 19 2014 /etc/init.d/mysql
-rw-r--r-- 1 root root 1770 Feb 19 2014 /etc/init/mysql.conf
$

This is the behavior for all packages using a current version of debhelper
to build.

Any packages that ship an upstart job and *do* have symlinks in /etc/rc?.d
for their matching init script are buggy and racy, and the change proposed
here will not make this problem worse.

> No /etc/rc?.d/ links
Of course, but I thought the point here was that one could/would call /etc/init.d/mysql directly when doing manual admin operations.

The first part of the patch, which disables an init.d script when called through /etc/rc?.d/ is correct; if this happens (upstart job with rc?.d symlink) this indeed is racy and undefined, but at the same time it should happen very rarely. The description/test case should mention this part, and fixing this in Trusty is prudent.

My objection is with manual admin operations via calling /etc/init.d/foo; that seems to be the main focus of the patch -- the second part beginning with UPSTART_JOB=${0#/etc/init.d/}. That's what the description and test case talk about, and they don't talk about the above race on boot.

We really don't know about all Ubuntu packages and even less about third party packages. Calling "/etc/init.d/foo restart" may have worked for people some packages, and it is neither racy nor undefined; the patch is going to change that behaviour, and this is definitively not guaranteed to be regression proof.

@Martin

Note that this is bug a regression - previously, before the (I presume) dh_installinit change, /etc/init.d/foo was a symlink to /lib/init/upstart-job so a user running /etc/init.d/foo by hand was redirected to upstart anyway. So while I appreciate that a user might now be getting desired behaviour by calling /etc/init.d/foo directly on some package somewhere, it seems unlikely that this change will regress by redirecting to the upstart job as we were doing for that package on a previous release - the window for that behaviour to have diverged is fairly small.

I sponsored this a while ago, unsubscribing sponsors. This is in the hands of the SRU team now.

Hello Eric, or anyone else affected,

Accepted lsb into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lsb/4.1+Debian11ubuntu6.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

I couldn't find any regression in my testing but since it wasn't that extensive, I'm not marking it a verified just yet.
I really like the behavior improvement. Now, a regular user has a convenient way to check service statuses:

  /etc/init.d/acpid status

Instead of the obtuse old way:

  env -u UPSTART_SESSION status acpid

Note: "service acpid status" still operates on the session upstart.

In Bug #1521771, I spent some time tracking down different behavior between the mysql-5.5 "init" and "upstart" scripts. They differ on how many seconds are waited between the SIGTERM and SIGKILL signals are sent. Different values can mean the difference between a slower clean shutdown and a shutdown interrupted by a SIGKILL signal.

In the case of that of that package, redirecting the "init.d" calls to the upstart script will have a positive impact in my opinion-- giving more time for MySQL to shutdown cleanly. So the proposed patch will be functionally helpful.

I do suggest that for any package that's affected by this, the "init.d" script should be cleaned out, so only the code remains that redirects people to the upstart script.

There is nothing about this line of code in an "init.d" script which indicates that that all the code below it is about to ignored:

  . /lib/lsb/init-functions

Nor does the proposed patch emit any output that confirms that redirect is happened. The result is that someone could pull their hair out wondering why the "init.d" script is not behaving as expected.

Realize that there are packages like "ec2-consistent-snapshot" which exist only in a PPA and make a hardcoded call to "/etc/init.d/mysql". It does that in hopes of working on non-Debian-based systems as well. (The package is also several years old, from an era when init.d scripts were more common). I'm not sure what small projects are supposed to when they want to issue a command like "stop mysql" in a way that might work across SysV init scripts, upstart and systemd.

On 3 December 2015 at 17:41, Mark Stosberg <email address hidden> wrote:
> In Bug #1521771, I spent some time tracking down different behavior
> between the mysql-5.5 "init" and "upstart" scripts. They differ on how
> many seconds are waited between the SIGTERM and SIGKILL signals are
> sent. Different values can mean the difference between a slower clean
> shutdown and a shutdown interrupted by a SIGKILL signal.
>
> In the case of that of that package, redirecting the "init.d" calls to
> the upstart script will have a positive impact in my opinion-- giving
> more time for MySQL to shutdown cleanly. So the proposed patch will be
> functionally helpful.
>
> I do suggest that for any package that's affected by this, the "init.d"
> script should be cleaned out, so only the code remains that redirects
> people to the upstart script.
>
> There is nothing about this line of code in an "init.d" script which
> indicates that that all the code below it is about to ignored:
>
> . /lib/lsb/init-functions
>
> Nor does the proposed patch emit any output that confirms that redirect
> is happened. The result is that someone could pull their hair out
> wondering why the "init.d" script is not behaving as expected.
>
> Realize that there are packages like "ec2-consistent-snapshot" which
> exist only in a PPA and make a hardcoded call to "/etc/init.d/mysql". It
> does that in hopes of working on non-Debian-based systems as well. (The
> package is also several years old, from an era when init.d scripts were
> more common). I'm not sure what small projects are supposed to when
> they want to issue a command like "stop mysql" in a way that might work
> across SysV init scripts, upstart and systemd.

Across all distros and init systems the following works, and does the
right thing, for the right init:

service mysql stop

--
Regards,

Dimitri.

Marking as verified on Trusty since I was able to do more testing.

Both mysql-server-5.5 and mysql-server-5.6 packages (e.g. under Trusty Thar) provide a sysvinit script and an upstart job for mysql. The problem here is that the maintainer has a very strange approach:

1. The sysvinit script links are symply removed in the postinstall script using update-rc.d
2.The sysvinit script doesn't respects Debian policy section 9.11.1 which states that the sysvinit scripts should abort when an upstart job is provided (As I know, Ubuntu has choosen to follow Debian policy regarding cooexistence of upstart jobs and sysvinit scripts)

So yes, this situation can lead to several problems.

Anybody that run the mysql sysvinit script directly (/etc/init.d/mysql) instead of using service mysql <action> will have problems.

Also, if the upstart job is present, and if the sysvninit script is enabled (sysvinit script links created), the mysql socket will be missing on reboot.

This bug was fixed in the package lsb - 4.1+Debian11ubuntu6.1

---------------
lsb (4.1+Debian11ubuntu6.1) trusty; urgency=medium

  * Add 01-upstart-lsb from the upstart package and make
    executing rc.d scripts no-op if there is an upstart
    job for that script. (LP: #1273462)

 -- Zhang Hua <email address hidden> Thu, 06 Aug 2015 11:11:16 -0500

The verification of the Stable Release Update for lsb has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.