Merge lp:~ev/apport/reports-from-hangs into lp:apport

[Evan]

This branch is not ready yet. I need to write tests (I know, I know, tests first) and build support in both the console and KDE interfaces. However, as I press on I want to make sure that you find the general approach to be sensible.

The branch adds the ability to specify that a provided pid is that of a hanging application. In the near future, compiz will call apport with these arguments when it detects that such a condition is occurring.

After review with the security team, it was determined that the safest approach to obtaining a stack trace from these hangs was through the core pipe handler. Therefore, the workflow is as follows:
- Compiz calls apport with --hanging $PID
- Apport collects some basic information to present a dialog:
https://wiki.ubuntu.com/ErrorTracker#When_there_is_an_error
- A key is added to the report denoting whether the application needs to be restarted, based on the user's response.
- Apport sends the application SIGSEGV.
- The apport core pipe handler picks this up and identifies that we already have a "Hang" report for this binary. It appends the core file to the existing report.
- update-notifier notices that the file has changed and calls apport-gtk.
- apport-gtk presents no UI, but restarts the application and tells whoopsie to send the report, if these options were selected.

So, what do you think?

Background:
https://bugs.launchpad.net/ubuntu/+source/whoopsie-daisy/+bug/1006398
https://bugs.launchpad.net/whoopsie/+bug/1015080
https://bugs.launchpad.net/whoopsie/+bug/1014716

[Martin Pitt (pitti)]

If compiz detects that it's hanging, it could just abort() and let the usual magic do its work. So I suppose this is primarily interesting for applications which need to ask the user "am I really broken"? Do we actually have an existing case where an application can detect that it is hanging? The only cases I've seen is that the kernel spits out some "process 12345 has not responded in 30 seconds" messages, but in this case it would probably be better to not rely on the application itself to call apport. I'd like to settle this question before we start adopting this rather complicated state machine.

Under the assumption that it indeed makes sense to have applications be able to self-detect that they are hung, but still able to call apport, this seems okay to me. One nitpick is that it would IMHO be better to kill them with SIGABRT.

Thanks for this!

[Evan (ev)]

The compiz binary detects that the application that it's painting for is hanging because it measures the time between glPaint calls (lp:compiz plugins/fade/src/fade.cpp:194). So calling abort() in compiz would kill compiz and generate an apport report for the compiz binary, not the hanging application.

Perhaps I'm missing something obvious, or misunderstanding how the window manager works? I agree that a complicated state machine is not ideal, and I am definitely open to alternatives.

Now, one could argue that compiz should send SIBABRT to the process, rather than making apport do it and thus simplifying the state that apport has to track. I would like to avoid this if possible. It means we cannot present the dialog while the application is still visible and it would require us to at least rework the text. "Force closed" would likely become "Leave closed", for example:

https://wiki.ubuntu.com/ErrorTracker#app-hang

Thanks Martin!

[Martin Pitt (pitti)]

Thanks for the explanations about how this is going to work, it makes much more sense now. I'm still pondering how we could avoid writing the report twice, with two different flags; this is so utterly prone to errors, if I had a dollar for each OSError/IOError and what not race condition I already fixed for those.. There might be half-written files, two parallel instances getting into each other's way, etc.

As we cannot augment a signal with additional data, nor intrude into the killed process' memory to set __abort_msg (that would require PTRACE permissions), the next best thing that comes to my mind is to not write a full report, but just an empty flag file into /var/crash that also contains the pid in the file name, which indicates to the next running apport instance that this was a manual SIGABRT due to a hang. That would be slightly more robust.

Otherwise I do not have a better idea as well.

Thanks!

[lp:~ev/apport/reports-from-hangs]

2399. By Evan on 2012-06-28

Rework hang reporting. We now drop a PID.hanging file in /var/crash, which the core pipe handler uses to assign a Hang problem type to the report it generates. This then hands back off to apport-gtk as before, but only marks the report for uploading. Restarting the application is now handled as the process is being terminated.

2400. By Evan on 2012-06-28

Merge with trunk.

2401. By Evan on 2012-06-28

It's hanging. It might not respond to SIGTERM. Send it SIGKILL instead.

[Evan (ev)]

Thanks again for the thorough review. I believe I've addressed the remaining issues by implementing your suggestion. The workflow is now as follows:

- Compiz detects a program is hanging and calls apport --hanging $PID.
- The apport UI is presented with some basic information about the hanging application.
- The user presses either the "Force closed" or "Relaunch" button.
- If the "report" box is checked, the ABRT signal is sent to the process and a /var/crash/$PID.hanging file will be written. Otherwise, just the KILL signal is sent to the process.
- If the user has pressed "Relaunch" then it is at this point that the process is restarted. This will occur after apport has waited for the previous pid to terminate.
- If the "report" box was checked and thus the ABRT signal was sent to the process, /usr/share/apport/apport will pick up the crash report. It will check for a /var/crash/$PID.hanging file that matches the pid it was just notified about. If it finds one, then it will create a "Hang" report rather than a "Crash" one.
- When apport-gtk is notified of new .crash files to process, it will check their problem type. For any that are "Hang" reports, it will simply mark the .crash file to be uploaded by whoopsie and then mark it as seen.

I'll write some tests and add support to the KDE and console frontends now. I'm not tied to the naming of anything. So if you think there's a better way of labelling any of these things, do let me know.

[lp:~ev/apport/reports-from-hangs]

2402. By Evan on 2012-06-28

Add tests for mark_report_upload and mark_hanging_process.

2403. By Evan on 2012-06-28

Add a test for wait_for_pid()

2404. By Evan on 2012-06-28

Added a test for the GTK layout of Hang problem reports.

[Evan (ev)]

Correction. It obviously does not make sense to add console support. Likewise, I am not sure what would be entailed in adding support to KDE's compositor, so I'd rather we revisit KDE frontend support when we have a plan there.

[lp:~ev/apport/reports-from-hangs]

2405. By Evan on 2012-06-29

Upgrade argv parsing tests with the new '--hanging' option.

2406. By Evan on 2012-06-29

Unit tests save the day. Fix accidentally treating successful loading of the crash report as a failure.

2407. By Evan on 2012-06-29

The PID option may not be set and does not have a default value.

[Evan (ev)]

I think this is finally ready to go. I've added some unit tests and corrected the new code where it was breaking existing ones. Let me know if you're happy with the result.

Thanks!

[Martin Pitt (pitti)]

Thanks Evan, this workflow seems good to me. I have one more concern:

9 + path = os.path.join(report_dir, '%s.hanging' % pid)

Can we still include the executable name and the uid here, for paranoia's sake? There might be race conditions about the user killing the process himself and then some other process starts and grabs the freed PID, or the hang is detected while the machine is shutting down (and thus all *.hanging files are invalid at the next boot). For addressing the latter I'd actually like to have the *.hanging files in /run/ somewhere, but a plausibility test in the UI (like comparing the .hanging mtime against /proc/uptime) would work fine as well.

Otherwise this looks fine to me, thanks!

review: Needs Fixing

[lp:~ev/apport/reports-from-hangs]

2408. By Evan on 2012-07-03

Use the ExecutablePath and uid as part of the .hanging path, to avoid potential race conditions.

2409. By Evan on 2012-07-03

Silently discard .hanging files that were created before a reboot as the pid is no longer valid.

2410. By Evan on 2012-07-03

Make the dialog modal for the hanging application when one is provided.

2411. By Evan on 2012-07-03

Remove double import.

[Evan (ev)]

I've made the requested changes and added some extra code to make the apport dialog modal to the hanging application.

[Martin Pitt (pitti)]

Thanks, great work! We need to remember to add a gir1.2-wnck build and binary dependency to the packaging.

Please merge to trunk (if all tests succeed).

review: Approve

[Martin Pitt (pitti)]

Ah, when you merge, please don't forget to commit the merge together with an appropriate NEWS entry.

[Evan (ev)]

All tests passed. Merged in with a NEWS entry.

=== modified file 'apport/fileutils.py'
--- apport/fileutils.py	2012-06-28 09:37:46 +0000
+++ apport/fileutils.py	2012-07-03 16:18:21 +0000
@@ -106,6 +106,19 @@
         pass
 
 
+def mark_hanging_process(report, pid):
+    if 'ExecutablePath' in report:
+        subject = report['ExecutablePath'].replace('/', '_')
+    else:
+        raise ValueError('report does not have the ExecutablePath attribute')
+
+    uid = os.getuid()
+    base = '%s.%s.%s.hanging' % (subject, str(uid), pid)
+    path = os.path.join(report_dir, base)
+    with open(path, 'a'):
+        pass
+
+
 def mark_report_seen(report):
     '''Mark given report file as seen.'''
 

=== modified file 'apport/ui.py'
--- apport/ui.py	2012-06-28 09:51:52 +0000
+++ apport/ui.py	2012-07-03 16:18:21 +0000
@@ -18,6 +18,8 @@
 import glob, sys, os.path, optparse, traceback, locale, gettext
 import errno, zlib
 import subprocess, threading, webbrowser
+import signal
+import time
 
 import apport, apport.fileutils, apport.REThread
 
@@ -191,7 +193,12 @@
         else:
             reports = apport.fileutils.get_new_reports()
         for f in reports:
-            self.run_crash(f)
+            if not self.load_report(f):
+                continue
+            if self.report['ProblemType'] == 'Hang':
+                self.finish_hang(f)
+            else:
+                self.run_crash(f)
             result = True
 
         return result
@@ -214,7 +221,7 @@
                 # not there any more? no problem, then it won't be regarded as
                 # "seen" any more anyway
                 pass
-            if not self.load_report(report_file):
+            if not self.report and not self.load_report(report_file):
                 return
 
             if 'Ignore' in self.report:
@@ -308,6 +315,63 @@
             else:
                 raise
 
+    def finish_hang(self, f):
+        '''Finish processing a hanging application after the core pipe handler
+        has handed the report back.
+
+        This will signal to whoopsie that the report needs to be uploaded.
+        '''
+        apport.fileutils.mark_report_upload(f)
+        apport.fileutils.mark_report_seen(f)
+
+    def run_hang(self, pid):
+        '''Report an application hanging.
+
+        This will first present a dialog containing the information it can
+        collect from the running application (everything but the trace) with
+        the option of terminating or restarting the application, optionally
+        reporting that this error occurred.
+
+        A SIGABRT will then be sent to the process and a series of
+        noninteractive processes will collect the remaining information and
+        mark the report for uploading.
+        '''
+        self.report = apport.Report('Hang')
+        self.report.add_proc_info(pid)
+        self.report.add_package_info()
+        path = self.report.get('ExecutablePath', '')
+        self.cur_package = apport.fileutils.find_file_package(path)
+        self.report.add_os_info()
+        allowed_to_report = apport.fileutils.allowed_to_report()
+        response = self.ui_present_report_details(allowed_to_report,
+                                                  modal_for=pid)
+        if response['report']:
+            apport.fileutils.mark_hanging_process(self.report, pid)
+            os.kill(int(pid), signal.SIGABRT)
+        else:
+            os.kill(int(pid), signal.SIGKILL)
+
+        if response['restart']:
+            self.wait_for_pid(pid)
+            self.restart()
+
+    def wait_for_pid(self, pid):
+        '''waitpid() does not work for non-child processes. Query the process
+        state in a loop, waiting for "no such process."
+        '''
+        while True:
+            try:
+                os.kill(int(pid), 0)
+            except OSError as e:
+                if e.errno == errno.ESRCH:
+                    break
+                else:
+                    raise
+            time.sleep(1)
+
+    def kill_segv(self, pid):
+        os.kill(int(pid), signal.SIGSEGV)
+
     def run_report_bug(self, symptom_script=None):
         '''Report a bug.
 
@@ -561,6 +625,9 @@
         if self.options.symptom:
             self.run_symptom()
             return True
+        elif hasattr(self.options, 'pid') and self.options.hanging:
+            self.run_hang(self.options.pid)
+            return True
         elif self.options.filebug:
             return self.run_report_bug()
         elif self.options.update_report is not None:
@@ -652,6 +719,8 @@
                              help=_('Specify package name in --file-bug mode. This is optional if a --pid is specified. (Implied if package name is given as only argument.)'))
         optparser.add_option('-P', '--pid', type='int',
                              help=_('Specify a running program in --file-bug mode. If this is specified, the bug report will contain more information.  (Implied if pid is given as only argument.)'))
+        optparser.add_option('--hanging', action='store_true', default=False,
+                             help=_('The provided pid is a hanging application.'))
         optparser.add_option('-c', '--crash-file', metavar='PATH',
                              help=_('Report the crash from given .apport or .crash file instead of the pending ones in %s. (Implied if file is given as only argument.)') % apport.fileutils.report_dir)
         optparser.add_option('--save', metavar='PATH',

=== modified file 'data/apport'
--- data/apport	2012-06-28 09:37:46 +0000
+++ data/apport	2012-07-03 16:18:21 +0000
@@ -329,6 +329,15 @@
         error_log('could not determine ExecutablePath, aborting')
         sys.exit(1)
 
+    subject = info['ExecutablePath'].replace('/', '_')
+    base = '%s.%s.%s.hanging' % (subject, str(pidstat.st_uid), pid)
+    hanging = os.path.join(apport.fileutils.report_dir, base)
+
+    if os.path.exists(hanging):
+        if (os.stat('/proc/uptime').st_ctime < os.stat(hanging).st_mtime):
+            info['ProblemType'] = 'Hang'
+        os.unlink(hanging)
+
     if 'InterpreterPath' in info:
         error_log('script: %s, interpreted by %s (command line "%s")' %
                   (info['ExecutablePath'], info['InterpreterPath'],

=== modified file 'gtk/apport-gtk'
--- gtk/apport-gtk	2012-06-21 13:38:27 +0000
+++ gtk/apport-gtk	2012-07-03 16:18:21 +0000
@@ -13,8 +13,7 @@
 
 import os.path, sys, subprocess, os, re
 
-from gi.repository import GObject
-from gi.repository import GLib
+from gi.repository import GObject, GLib, Wnck, GdkX11, Gdk
 try:
     from gi.repository import Gtk
 except RuntimeError as e:
@@ -25,7 +24,17 @@
 import apport
 from apport import unicode_gettext as _
 import apport.ui
-from gi.repository import Gdk
+
+def find_xid_for_pid (pid):
+    '''Return the X11 Window (xid) for the supplied process ID.'''
+
+    pid = int(pid)
+    screen = Wnck.Screen.get_default()
+    screen.force_update()
+    for window in screen.get_windows():
+        if window.get_pid() == pid:
+            return window.get_xid()
+    return None
 
 class GTKUserInterface(apport.ui.UserInterface):
     '''GTK UserInterface.'''
@@ -175,7 +184,16 @@
         self.w('send_error_report').set_active(True)
         self.w('send_error_report').hide()
 
-    def ui_present_report_details(self, allowed_to_report=True):
+    def set_modal_for(self, xid):
+        gdk_window = self.w('dialog_crash_new')
+        gdk_window.realize()
+        gdk_window = gdk_window.get_window()
+        gdk_display = GdkX11.X11Display.get_default()
+        foreign = GdkX11.X11Window.foreign_new_for_display(gdk_display, xid)
+        gdk_window.set_transient_for(foreign)
+        gdk_window.set_modal_hint(True)
+
+    def ui_present_report_details(self, allowed_to_report=True, modal_for=None):
         icon = None
         self.collect_called = False
         report_type = self.report.get('ProblemType')
@@ -195,7 +213,30 @@
         self.w('continue_button').set_label(_('Continue'))
 
         self.w('cancel_button').hide()
-        if not self.report_file:
+        if modal_for is not None:
+            xid = find_xid_for_pid(modal_for)
+            if xid:
+                self.set_modal_for(xid)
+
+        if report_type == 'Hang':
+            self.w('ignore_future_problems').set_active(False)
+            self.w('ignore_future_problems').hide()
+            self.w('closed_button').show()
+            self.w('closed_button').set_label(_('Force Closed'))
+            self.w('continue_button').set_label(_('Relaunch'))
+            self.w('subtitle_label').hide()
+            self.desktop_info = self.get_desktop_entry()
+            if self.desktop_info:
+                icon = self.desktop_info.get('icon')
+                name = self.desktop_info['name']
+                name = GLib.markup_escape_text(name)
+                title = _('The application %s has stopped responding.') % name
+            else:
+                icon = 'distributor-logo'
+                name = os.path.basename(self.report['ExecutablePath'])
+                title = _('The program "%s" has stopped responding.') % name
+            self.w('title_label').set_label('<big><b>%s</b></big>' % title)
+        elif not self.report_file:
             self.setup_bug_report()
         elif report_type == 'KernelCrash' or report_type == 'KernelOops':
             self.w('ignore_future_problems').set_active(False)

=== modified file 'test/test_fileutils.py'
--- test/test_fileutils.py	2012-06-28 09:37:46 +0000
+++ test/test_fileutils.py	2012-07-03 16:18:21 +0000
@@ -113,6 +113,23 @@
             self.assertEqual(apport.fileutils.seen_report(r), True)
             self.assertEqual(set(apport.fileutils.get_new_reports()), nr)
 
+    def test_mark_hanging_process(self):
+        '''mark_hanging_process()'''
+        pr = problem_report.ProblemReport()
+        pr['ExecutablePath'] = '/bin/bash'
+        apport.fileutils.mark_hanging_process(pr, '1')
+        uid = str(os.getuid())
+        base = '_bin_bash.%s.1.hanging' % uid
+        expected = os.path.join(apport.fileutils.report_dir, base)
+        self.assertTrue(os.path.exists(expected))
+
+    def test_mark_report_upload(self):
+        '''mark_report_upload()'''
+        report = os.path.join(apport.fileutils.report_dir, 'report.crash')
+        apport.fileutils.mark_report_upload(report)
+        expected = os.path.join(apport.fileutils.report_dir, 'report.upload')
+        self.assertTrue(os.path.exists(expected))
+
     def test_get_all_reports(self):
         '''get_all_reports()'''
 

=== modified file 'test/test_ui.py'
--- test/test_ui.py	2012-06-28 09:37:46 +0000
+++ test/test_ui.py	2012-07-03 16:18:21 +0000
@@ -183,7 +183,7 @@
         self.ui = None
         self.report_file.close()
 
-        self.assertEqual(subprocess.call(['pidof', '/bin/yes']), 1, 'no stray test processes')
+        self.assertEqual(subprocess.call(['pidof', '/usr/bin/yes']), 1, 'no stray test processes')
 
         # clean up apport report from _gen_test_crash()
         for f in glob.glob('/var/crash/_usr_bin_yes.*.crash'):
@@ -1711,7 +1711,7 @@
         _chk('apport-gtk', None,
              {'filebug': False, 'package': None, 'pid': None, 'crash_file':
               None, 'symptom': None, 'update_report': None, 'save': None,
-              'window': False, 'tag': []})
+              'window': False, 'tag': [], 'hanging': False})
         # updating report not allowed without args
         self.assertRaises(SystemExit, _chk, 'apport-collect', None, {})
 
@@ -1719,7 +1719,7 @@
         _chk('apport-kde', 'coreutils',
              {'filebug': True, 'package': 'coreutils', 'pid': None,
               'crash_file': None, 'symptom': None, 'update_report': None,
-              'save': None, 'window': False, 'tag': []})
+              'save': None, 'window': False, 'tag': [], 'hanging': False})
 
         # symptom is preferred over package
         f = open(os.path.join(apport.ui.symptom_script_dir, 'coreutils.py'), 'w')
@@ -1731,13 +1731,13 @@
         _chk('apport-cli', 'coreutils',
              {'filebug': True, 'package': None, 'pid': None, 'crash_file':
               None, 'symptom': 'coreutils', 'update_report': None, 'save':
-              None, 'window': False, 'tag': []})
+              None, 'window': False, 'tag': [], 'hanging': False})
 
         # PID
         _chk('apport-cli', '1234', {'filebug': True, 'package': None,
              'pid': '1234', 'crash_file': None, 'symptom': None,
              'update_report': None, 'save': None, 'window': False,
-             'tag': []})
+             'tag': [], 'hanging': False})
 
         # .crash/.apport files; check correct handling of spaces
         for suffix in ('.crash', '.apport'):
@@ -1745,14 +1745,14 @@
                  'package': None, 'pid': None,
                  'crash_file': '/tmp/f oo' + suffix, 'symptom': None,
                  'update_report': None, 'save': None, 'window': False,
-                 'tag': []})
+                 'tag': [], 'hanging': False})
 
         # executable
         _chk('apport-cli', '/usr/bin/tail', {'filebug': True,
              'package': 'coreutils',
              'pid': None, 'crash_file': None, 'symptom': None,
              'update_report': None, 'save': None, 'window': False,
-             'tag': []})
+             'tag': [], 'hanging': False})
 
         # update existing report
         _chk('apport-collect', '1234', {'filebug': False, 'package': None,
@@ -1783,7 +1783,7 @@
         #
         _chk([], {'filebug': True, 'package': None, 'pid': None, 'crash_file':
                   None, 'symptom': None, 'update_report': None, 'save': None,
-                  'window': False, 'tag': []})
+                  'window': False, 'tag': [], 'hanging': False})
 
         #
         # single arguments
@@ -1793,7 +1793,7 @@
         _chk(['coreutils'], {'filebug': True, 'package': 'coreutils', 'pid':
                              None, 'crash_file': None, 'symptom': None,
                              'update_report': None, 'save': None, 'window':
-                             False, 'tag': []})
+                             False, 'tag': [], 'hanging': False})
 
         # symptom (preferred over package)
         f = open(os.path.join(apport.ui.symptom_script_dir, 'coreutils.py'), 'w')
@@ -1805,26 +1805,27 @@
         _chk(['coreutils'], {'filebug': True, 'package': None, 'pid': None,
                              'crash_file': None, 'symptom': 'coreutils',
                              'update_report': None, 'save': None, 'window':
-                             False, 'tag': []})
+                             False, 'tag': [], 'hanging': False})
         os.unlink(os.path.join(apport.ui.symptom_script_dir, 'coreutils.py'))
 
         # PID
         _chk(['1234'], {'filebug': True, 'package': None, 'pid': '1234',
                         'crash_file': None, 'symptom': None, 'update_report':
-                        None, 'save': None, 'window': False, 'tag': []})
+                        None, 'save': None, 'window': False, 'tag': [],
+                        'hanging': False})
 
         # .crash/.apport files; check correct handling of spaces
         for suffix in ('.crash', '.apport'):
             _chk(['/tmp/f oo' + suffix],
                  {'filebug': False, 'package': None, 'pid': None, 'crash_file':
                   '/tmp/f oo' + suffix, 'symptom': None, 'update_report': None,
-                  'save': None, 'window': False, 'tag': []})
+                  'save': None, 'window': False, 'tag': [], 'hanging': False})
 
         # executable name
         _chk(['/usr/bin/tail'],
              {'filebug': True, 'package': 'coreutils', 'pid': None,
               'crash_file': None, 'symptom': None, 'update_report': None,
-              'save': None, 'window': False, 'tag': []})
+              'save': None, 'window': False, 'tag': [], 'hanging': False})
 
         #
         # supported options
@@ -1834,17 +1835,19 @@
         _chk(['--save', 'foo.apport', 'coreutils'],
              {'filebug': True, 'package': 'coreutils', 'pid': None,
               'crash_file': None, 'symptom': None, 'update_report': None,
-              'save': 'foo.apport', 'window': False, 'tag': []})
+              'save': 'foo.apport', 'window': False, 'tag': [],
+              'hanging': False})
 
         # --tag
         _chk(['--tag', 'foo', 'coreutils'],
              {'filebug': True, 'package': 'coreutils', 'pid': None,
               'crash_file': None, 'symptom': None, 'update_report': None,
-              'save': None, 'window': False, 'tag': ['foo']})
+              'save': None, 'window': False, 'tag': ['foo'], 'hanging': False})
         _chk(['--tag', 'foo', '--tag', 'bar', 'coreutils'],
              {'filebug': True, 'package': 'coreutils', 'pid': None,
               'crash_file': None, 'symptom': None, 'update_report': None,
-              'save': None, 'window': False, 'tag': ['foo', 'bar']})
+              'save': None, 'window': False, 'tag': ['foo', 'bar'],
+              'hanging': False})
 
     def test_can_examine_locally_crash(self):
         '''can_examine_locally() for a crash report'''
@@ -1948,5 +1951,20 @@
                                 'genericname[de]': 'Übersetzer',
                                 'exec': 'gedit %U'})
 
+    def test_wait_for_pid(self):
+        # fork a test process
+        pid = os.fork()
+        if pid == 0:
+            os.dup2(os.open('/dev/null', os.O_WRONLY), sys.stdout.fileno())
+            os.execv('/usr/bin/yes', ['yes'])
+            assert False, 'Could not execute /usr/bin/yes'
+
+        time.sleep(0.5)
+        os.kill(pid, signal.SIGKILL)
+        os.waitpid(pid, 0)
+        self.ui.wait_for_pid(pid)
+
+        self.assertRaises(OSError, self.ui.wait_for_pid, '1')
+
 
 unittest.main()

=== modified file 'test/test_ui_gtk.py'
--- test/test_ui_gtk.py	2012-06-28 09:37:46 +0000
+++ test/test_ui_gtk.py	2012-07-03 16:18:21 +0000
@@ -227,6 +227,44 @@
         self.assertTrue(self.app.w('ignore_future_problems').get_label().endswith(
             'of this program version'))
 
+    def test_hang_layout(self):
+        '''
+        +-----------------------------------------------------------------+
+        | [ apport ] The application Apport has stopped responding.       |
+        |                                                                 |
+        |            [x] Send an error report to help fix this problem.   |
+        |                                                                 |
+        | [ Show Details ]                 [ Force Closed ]  [ Relaunch ] |
+        +-----------------------------------------------------------------+
+        '''
+        self.app.report['ProblemType'] = 'Hang'
+        self.app.report['ProcCmdline'] = 'apport-bug apport'
+        self.app.report['Package'] = 'apport 1.2.3~0ubuntu1'
+        with tempfile.NamedTemporaryFile() as fp:
+            fp.write(b'''[Desktop Entry]
+Version=1.0
+Name=Apport
+Type=Application''')
+            fp.flush()
+            self.app.report['DesktopFile'] = fp.name
+            GLib.idle_add(Gtk.main_quit)
+            self.app.ui_present_report_details(True)
+        self.assertEqual(self.app.w('dialog_crash_new').get_title(), self.distro)
+        self.assertEqual(self.app.w('title_label').get_text(),
+                         _('The application Apport has stopped responding.'))
+        send_error_report = self.app.w('send_error_report')
+        self.assertTrue(send_error_report.get_property('visible'))
+        self.assertTrue(send_error_report.get_active())
+        self.assertTrue(self.app.w('show_details').get_property('visible'))
+        self.assertTrue(self.app.w('continue_button').get_property('visible'))
+        self.assertEqual(self.app.w('continue_button').get_label(),
+                         _('Relaunch'))
+        self.assertTrue(self.app.w('closed_button').get_property('visible'))
+        self.assertEqual(self.app.w('closed_button').get_label(),
+                         _('Force Closed'))
+        self.assertFalse(self.app.w('subtitle_label').get_property('visible'))
+        self.assertFalse(self.app.w('ignore_future_problems').get_property('visible'))
+
     def test_system_crash_layout(self):
         '''
         +---------------------------------------------------------------+