Reject crashes that happen right after upgrade

[Martin Pitt]

According to Sebastian we often get crash reports for packages which just have been upgraded, but the old version of the executable is still running. (e. g. bug 983697). Apport already prevents filing a report when the executable changed between the time the crash happens and the time the user reports the bug, but this is not sufficient here: In that example, evince crashed right _after_ installing the new version (the running process got confused about the new data on disk presumably), so that check doesn't help.

We need to check if the binary was modified after the process started.

FIX: http://bazaar.launchpad.net/~apport-hackers/apport/trunk/revision/2296

SRU TEST CASE:
 - Install an older version of shotwell, e. g. the version from precise final.
 - Start "shotwell &" in a terminal (or any other program, really)
 - upgrade shotwell to precise-updates.
 - Let it crash with "killall -SEGV shotwell"
 - In the precise version, apport either displays the bug, or /var/log/apport.log gets an exception about self['ExecutablePath'] not existing any more.
 - In this fixed version, apport.log just gets "executable was modified after program start, ignoring" and does not produce a report in /var/crash/.
 - Without upgrading the package in between, the proposed version continues to generate a proper crash report.

REGRESSION POTENTIAL: Low, the change has been tested in quantal for a while already, is covered by a test case, and the other dozens of test cases cover the "normal" case when a binary is older than the process start time.

[Martin Pitt (pitti)]

/proc/pid/status does not have an obvious start time, but we can look at the mtime of the /proc/pid/exe symlink and compare it with the mtime of the symlink target.

[Martin Pitt (pitti)]

Fixed in trunk r2296.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.1-0ubuntu1

---------------
apport (2.1-0ubuntu1) quantal; urgency=low

  * New upstream release:
    - packaging.py, install_packages(): Add permanent_rootdir flag and if set,
      only unpack newly downloaded packages. Implement it for the apt/dpkg
      backend. Thanks Evan Dandrea.
    - apport-retrace: Add --sandbox-dir option for keeping a permanent sandbox
      (unpacked packages). This provides a considerable speedup. Thanks Evan
      Dandrea.
    - crash-digger: Add --sandbox-dir option and pass it to apport-retrace.
    - Fix the whole code to be PEP-8 compatible, and enforce this in test/run
      by running the "pep8" tool.
    - GTK UI tests: Ensure that there are no GLib/GTK warnings or criticals.
    - Support Python 3. Everything except the launchpad crashdb backend now
      works with both Python 2 and 3. An important change is that the load(),
      write(), and write_mime() methods of a ProblemReport and apport.Report
      object now require the file stream to be opened in binary mode.
    - data/apport: Ignore a crash if the executable was modified after the
      process started. This often happens if the package is upgraded and a
      long-running process is not stopped before. (LP: #984944)
    - Add test cases for apport-unpack.
    - apport-retrace: Add information about outdated packages to the
      "RetraceOutdatedPackages" field.
    - ui.py: Drop python-xdg dependency, use ConfigParser to read the .desktop
      files.
    - apport-gtk: Work around GTK crash when trying to set pixmap on an
      already destroyed parent window. (LP: #938090)
    - data/dump_acpi_tables.py: Fix crash on undefined variable with
      non-standard tables. (LP: #982267)
    - backends/packaging-apt-dpkg.py: Fix crash if a package is installed, but
      has no candidates in apt. (LP: #980094)
    - data/general-hooks/generic.py: Bump minimum free space requirement from
      10 to 50 MB. 10 is not nearly enough particularly for /tmp. (LP: #979928)
    - hookutils.py, recent_logfile(): Use a default limit of 10000 lines and
      call "tail" instead of reading the whole file. This protects against
      using up all memory when there are massive repeated log messages.
      (LP: #984256)
    - apport-gtk: Do not assume that an icon requested for size 42 actually
      delivers size 42; some themes do not have this available and deliver a
      smaller one instead, causing overflows. Also, copy the image as
      gtk_icon_theme_load_icon() returns a readonly result which we must not
      modify. (LP: #937249)
    - ui.py: Don't show the duplicate warning when the crash database does not
      accept the problem type, and they are just being sent to whoopsie.
      Thanks Evan Dandrea. (LP: #989779)
    - report.py: Correctly escape the file path passed to gdb.
    - apport-gtk, apport-kde: Do not show the information collection progress
      dialog if the crash database does not accept this kind of report. In that
      case whoopsie will upload it in the background and the dialog is not
      necessary. (LP: #989698)
  * data/general-hooks/ubuntu.py, data/general-hooks/automatix.py...

[Clint Byrum (clint-fewbar)]

Hi! This bug needs a test case and regression potential before we accept the upload to precise-proposed.

[Brian Murray (brian-murray)]

Hello Martin, or anyone else affected,

Accepted apport into precise-proposed. The package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[John S. Gruber (jsjgruber)]

A problem was indicated in installing python-problem-report_2.0.1-0ubuntu9_all.deb at http://askubuntu.com/questions/148383/how-to-resolve-dpkg-error-processing-var-cache-apt-archives-python-apport-2-0/148393#148393. I believe that is this SRU.

Of course this could simply have been a user error in using the proposed archive or something else entirely. FYI.

[Sebastien Bacher (seb128)]

using the testcase the log get those:

"ERROR: apport (pid 9970) Tue Jun 12 12:50:37 2012: Unhandled exception:
Traceback (most recent call last):
  File "/usr/share/apport/apport", line 290, in <module>
    info.add_proc_info(pid)
  File "/usr/lib/python2.7/dist-packages/apport/report.py", line 439, in add_proc_info
    assert os.path.exists(self['ExecutablePath'])
AssertionError"

rather than the described "executable was modified after program start, ignoring", the bug is not recorded though so it seems that still fix the issue even if that's not the wanted way?

[Martin Pitt (pitti)]

That's actually not expected. While the exception certainly helps to prevent the bug filing (see the test case description), it's supposed to not even get that far, as the mtime comparison happens earlier. So I guess that test case is wrong, we probably need one which actually involves upgrading a package. I'll look into that.

[Martin Pitt (pitti)]

Indeed, a mere touch would not actually change the executable; a package upgrade would change the inode, so they don't test the same thing.

[Sebastien Bacher (seb128)]

I still can't verify that, testing with the package udpate steps I still get

Traceback (most recent call last):
  File "/usr/share/apport/apport", line 290, in <module>
    info.add_proc_info(pid)
  File "/usr/lib/python2.7/dist-packages/apport/report.py", line 439, in add_proc_info
    assert os.path.exists(self['ExecutablePath'])

[Martin Pitt (pitti)]

I just realized that gnomine is a hard example, as it forks, and thus killall will kill both processes. So you either manage to kill the right one, or apport will race with itself, and you might catch the wrong process first.

So you get the exception because in this case the symlink shows "... (deleted)" and thus the stat only stats the /exe symlink instead of the target binary. The followup exception catches this and still prevents the bug from being reported, so this scenario actually already works in precise final. But I can easily clean this up to work without an exception.

[Martin Pitt (pitti)]

I uploaded 2.0.1-0ubuntu11 to the precise-proposed unapproved queue now which fixes this in a cleaner way.

[Brian Murray (brian-murray)]

Hello Martin, or anyone else affected,

Accepted apport into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/apport/2.0.1-0ubuntu11 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 change the bug tag from verification-needed to verification-done. If it does not, 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!

[Stéphane Graber (stgraber)]

ERROR: apport (pid 4123) Mon Jul 16 17:37:12 2012: called for pid 3742, signal 11
ERROR: apport (pid 4123) Mon Jul 16 17:37:12 2012: executable was modified after program start, ignoring

Looks good, verification-done

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.0.1-0ubuntu11

---------------
apport (2.0.1-0ubuntu11) precise-proposed; urgency=low

  * data/package-hooks/source_linux.py: If we report against an -lts-quantal
    source package, move the source to "linux" and add a qa-kernel-lts-testing
    tag, as per kernel team request in LP: #1004101 . Add source package hook
    symlinks for source_linux-{,meta-}lts-quantal.py to source_linux.py.
  * data/apport: apport: Also treat a binary as modified if the /proc/pid/exe
    symlink does not point to an existing file any more. Backported from trunk
    r2406. (LP: #984944)

apport (2.0.1-0ubuntu10) precise-proposed; urgency=low

  * debian/apport.install: Actually ship the native-origins.d directory, so
    that the previous bug fix for LP: #1004101 actually works.

apport (2.0.1-0ubuntu9) precise-proposed; urgency=low

  [ Martin Pitt ]
  * data/apport: Ignore a crash if the executable was modified after the
    process started. This often happens if the package is upgraded and a
    long-running process is not stopped before. Patch cherry-picked from trunk
    r2296. (LP: #984944)
  * Add etc/apport/native-origins.d/lts-q-backports: Accept
    ppa:ubuntu-x-swat/q-lts-backport as official Ubuntu package repository, so
    that users can report bugs and crashes against the backported kernel and
    X.org stack. (LP: #1004101)
  * data/general-hooks/ubuntu.py: Do not assume that all reports have a
    ProblemType field. This will not be the case for updating a bug with
    "apport-collect". (LP: #1004029)
  * report.py: Do not change the SourcePackage: field if the binary package is
    not installed and does not exist. This fixes source package hooks to
    actually work in some cases where source and binary package names overlap.
    Patch cherry-picked from trunk r2332. (part of LP: #993810)
  * apport-gtk, apport-kde: Avoid collecting information twice in "bug update"
    mode. This caused a crash in cases where the source package in a bug
    report does not correspond to an installed binary package. Patch
    cherry-picked from trunk r2334. (LP: #993810)

  [ Brian Murray ]
  * data/general-hooks/ubuntu.py: block reporting of package install failures
    with error regarding 'not a debian format archive'. (LP: #1002535)
 -- Martin Pitt <email address hidden> Thu, 28 Jun 2012 09:01:41 +0200