/etc/init/apport-noui.conf is non-functional on the phone

Bug #1235436 reported by Steve Langasek
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
apport (Ubuntu)
Fix Released
Critical
Brian Murray

Bug Description

I've uploaded a fix to upstart for bug #1235387, but /etc/init/apport-noui.conf in the apport-noui package still doesn't work here. There's no /etc/apport/autoreport in the image, so it fails on the first line of the script:

    [ -e /etc/apport/autoreport ] || exit 0

Apparently further integration work is needed.

The job is also structured quite confusingly, with an 'instance' that means the job will be launched once for each file found in /var/crash - even though the command it runs is called /usr/share/apport/whoopsie-upload-*all*. So we'll get multiple copies of this running in parallel the first time it successfully triggers. I suspect this may result in multiple submissions, as whoopsie sees the .upload stamp updated?

In any case, I don't see the point of having this as a separate upstart job from whoopsie at all. whoopsie is already monitoring the /var/crash directory with inotify; it should just check /etc/apport/autoreport itself (or maybe this should be /etc/whoopsie/autoreport?), and if present, monitor the .crash files directly.

Steve Langasek (vorlon)
Changed in apport (Ubuntu):
importance: Undecided → High
Revision history for this message
Steve Langasek (vorlon) wrote :

So just to confirm, after manually touching /etc/apport/autoreport, my phone is now being crushed by several instances of the apport-noui job processing *the same crash file*, in parallel - apparently, for some reason whoopsie-upload-all finds it necessary to run gdb over the crash files, which is not pretty.

This reinforces my belief that we should ditch /etc/init/apport-noui.conf completely, and let whoopsie handle those crash files directly.

But it also looks to me like /usr/share/apport/apport does not write its crash files atomically... it opens the .crash file, writes to it, and closes it. Which means that regardless of who's watching /var/crash with inotify, there's a race condition here that means some things may try to process the crash files before they're fully written. And on armhf, writing these crash files out is sometimes VERY slow. I think we need apport fixed to write to a temp file and atomically rename into place as .crash, to ensure this all works reliably.

Steve Langasek (vorlon)
Changed in apport (Ubuntu):
assignee: nobody → Evan Dandrea (ev)
Evan (ev)
Changed in apport (Ubuntu):
importance: High → Critical
milestone: none → ubuntu-13.10
Revision history for this message
Martin Pitt (pitti) wrote :

> But it also looks to me like /usr/share/apport/apport does not write its crash files atomically... it opens the .crash file, writes to it, and closes it.

FTR, It opens the file with mode 0, and only chmods it to 600 once it's done writing (after closing). By any other method, inotify watches would also be triggered, so it seems best to only consider crash files which you can actually access.

Changed in apport (Ubuntu):
milestone: ubuntu-13.10 → saucy-updates
Revision history for this message
Steve Langasek (vorlon) wrote :

missed the boat on this for 13.10; most likely doesn't warrant an SRU since we're aiming to focus all our phone development efforts on trusty.

Changed in apport (Ubuntu):
milestone: saucy-updates → ubuntu-14.04
Revision history for this message
Brian Murray (brian-murray) wrote :

If the location of /etc/apport/autoreport changes, which it should since /etc/ is read-only on the phone images, then whoopsie-preferences needs to have the location updated too. This should end up fixing the following bug report:

https://bugs.launchpad.net/ubuntu/+source/whoopsie-preferences/+bug/1239811

Changed in apport (Ubuntu):
assignee: Evan Dandrea (ev) → Brian Murray (brian-murray)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apport - 2.12.7-0ubuntu6

---------------
apport (2.12.7-0ubuntu6) trusty; urgency=medium

  * Merge from trunk:
    - setup.py: Make updating of hashbangs work when building without Java,
      and also apply it on bin/.
  * Bump Standards-Version to 3.9.5, no changes necessary.
 -- Martin Pitt <email address hidden> Tue, 07 Jan 2014 18:41:12 +0100

Changed in apport (Ubuntu):
status: New → Fix Released
Revision history for this message
Martin Pitt (pitti) wrote :

So Brian changed /etc/apport/autoreport to /var/lib/apport/autoreport, and added that path in

lxc-android-config (0.126) trusty; urgency=low

  * Adding /var/lib/apport to writable persistent paths. (LP: #1239811)

 -- Brian Murray <email address hidden> Mon, 06 Jan 2014 12:03:20 -0800

Why can't we use /etc/ for this instead? This *is* a configuration file after all, and moving it to /var/lib/ is just a bad hack to work aorund our bad hack of non-writable /etc/. Can't we keep that in /etc/apport/ and make /etc/apport/autoreport writable instead?

Revision history for this message
Steve Langasek (vorlon) wrote : Re: [Bug 1235436] Re: /etc/init/apport-noui.conf is non-functional on the phone

On Thu, Jan 09, 2014 at 11:50:14AM -0000, Martin Pitt wrote:
> So Brian changed /etc/apport/autoreport to /var/lib/apport/autoreport,
> and added that path in

> lxc-android-config (0.126) trusty; urgency=low

> * Adding /var/lib/apport to writable persistent paths. (LP: #1239811)

> -- Brian Murray <email address hidden> Mon, 06 Jan 2014 12:03:20 -0800

> Why can't we use /etc/ for this instead? This *is* a configuration file
> after all, and moving it to /var/lib/ is just a bad hack to work aorund
> our bad hack of non-writable /etc/. Can't we keep that in /etc/apport/
> and make /etc/apport/autoreport writable instead?

I asked him to do this because I consider bind-mounting of individual files
a horrible hack. Since this is a configuration setting that's going to be
changed almost exclusively through the phone UI anyway, there's no logical
advantage of considering this a config file rather than "state", as it
should not be a conffile.

Revision history for this message
Martin Pitt (pitti) wrote :

It has pretty much the same purpose and semantics like an /etc/default/ file. On a desktop it should very much be a config file (not a conffile). It seems to me it only is not any more because our phones have a broken /etc/, not because we actually don't want it to be configuration?

Revision history for this message
Steve Langasek (vorlon) wrote :

On Thu, Jan 09, 2014 at 04:56:00PM -0000, Martin Pitt wrote:
> It has pretty much the same purpose and semantics like an /etc/default/
> file. On a desktop it should very much be a config file (not a
> conffile). It seems to me it only is not any more because our phones
> have a broken /etc/, not because we actually don't want it to be
> configuration?

In general, if the primary way the file is edited is from the software and
not via using an editor on the filesystem, I don't think there's a strong
argument that the file belongs in /etc.

Revision history for this message
Oliver Grawert (ogra) wrote :

this was never fixed, apport needs to create the dir from postinst or ship it in debian/apport.dirs, writable-paths only works with existing directories by design, so the change to lxc-android-config was a no-op.

additionally there are also the lines:

env MATCH=NULL
 [ "$MATCH" = NULL ] && exit 0

in the apport-noui.conf job, so it would always exit anyway.

Changed in apport (Ubuntu):
status: Fix Released → Confirmed
Revision history for this message
Brian Murray (brian-murray) wrote :

While I've addressed oliver's comments from comment #10 there is still no user interface for the setting of automatic crash reporting and I've reported this as bug 1319516. However, if you now manually create /var/lib/apport/autoreport most crash reports will be automatically sent.

Revision history for this message
Brian Murray (brian-murray) wrote :

I say most because of bug 1319099 which I'll be working on soon.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apport - 2.14.2-0ubuntu3

---------------
apport (2.14.2-0ubuntu3) utopic; urgency=medium

  * debian/apport-noui.upstart: remove early exit (LP: #1235436)
  * debian/apport-noui.dirs: create /var/lib/apport (LP: #1235436)
 -- Brian Murray <email address hidden> Wed, 14 May 2014 12:26:39 -0700

Changed in apport (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.