From 64ee54fea156694844a720f9b11d78e2e95d64ad Mon Sep 17 00:00:00 2001 From: Christoph Reiter Date: Mon, 27 Feb 2017 08:23:16 +0100 Subject: [PATCH] Better integration of faulthandler. See #1853 Instead of writing the stacktrace to stderr, write it to ~/.quodlibet/faultdump and on the next start raise an exception with the content. Slightly adjust the exception dialog to use a text view for the exception message instead of a tree view header since the passed text from the faulthandler stack trace can be quite long. --- quodlibet/quodlibet/_init.py | 8 -- quodlibet/quodlibet/_main.py | 16 ++- quodlibet/quodlibet/qltk/debugwindow.py | 11 ++- quodlibet/quodlibet/util/faulthandling.py | 110 +++++++++++++++++++++ quodlibet/tests/helper.py | 7 +- quodlibet/tests/test_util_faulthandling.py | 20 ++++ 6 files changed, 160 insertions(+), 12 deletions(-) create mode 100644 quodlibet/quodlibet/util/faulthandling.py create mode 100644 quodlibet/tests/test_util_faulthandling.py diff --git a/quodlibet/quodlibet/_init.py b/quodlibet/quodlibet/_init.py index f36b992cbe..f102b64449 100644 --- a/quodlibet/quodlibet/_init.py +++ b/quodlibet/quodlibet/_init.py @@ -38,14 +38,6 @@ def _override_exceptions(): if not no_excepthook: GLib.idle_add(_override_exceptions) - # faulthandler gives a python stacktrace on segfaults.. - try: - import faulthandler - except ImportError: - pass - else: - faulthandler.enable() - def is_init(): """Returns if init() was called""" diff --git a/quodlibet/quodlibet/_main.py b/quodlibet/quodlibet/_main.py index 49d8c870d3..e79d36665a 100644 --- a/quodlibet/quodlibet/_main.py +++ b/quodlibet/quodlibet/_main.py @@ -16,6 +16,7 @@ from quodlibet.util import cached_func, windows, set_process_title from quodlibet.util.dprint import print_d from quodlibet.util.path import mkdir +from quodlibet.util import faulthandling PLUGIN_DIRS = ["editing", "events", "playorder", "songsmenu", "playlist", @@ -272,7 +273,7 @@ def application_openFile_(self, sender, filename): def run(window, before_quit=None): print_d("Entering quodlibet.main") - from gi.repository import Gtk, Gdk + from gi.repository import Gtk, Gdk, GLib from quodlibet._init import is_init assert is_init() @@ -319,6 +320,17 @@ def quit_gtk(window): # if we don't show a window, startup isn't completed, so call manually Gdk.notify_startup_complete() + try: + faulthandling.enable(os.path.join(get_user_dir(), "faultdump")) + crash_message = faulthandling.check_and_clear_error() + except IOError: + util.print_exc() + else: + if crash_message is not None: + def trigger_crash_exception(): + raise Exception(crash_message) + GLib.idle_add(trigger_crash_exception) + # set QUODLIBET_START_PERF to measure startup time until the # windows is first shown. if "QUODLIBET_START_PERF" in environ: @@ -328,6 +340,8 @@ def quit_gtk(window): else: Gtk.main() + faulthandling.disable() + print_d("Gtk.main() done.") diff --git a/quodlibet/quodlibet/qltk/debugwindow.py b/quodlibet/quodlibet/qltk/debugwindow.py index 975b519527..d94671aa78 100644 --- a/quodlibet/quodlibet/qltk/debugwindow.py +++ b/quodlibet/quodlibet/qltk/debugwindow.py @@ -183,11 +183,18 @@ def __init__(self, type_, value, traceback): label.set_line_wrap(True) box = Gtk.VBox(spacing=6) buttons = Gtk.HButtonBox() + + viewbox = Gtk.VBox(spacing=4) + buf = Gtk.TextBuffer() + buf.set_text(text_type(value)) + viewbox.add(Gtk.TextView(buffer=buf, editable=False)) view = Gtk.TreeView() + viewbox.add(view) + view.set_headers_visible(False) sw = Gtk.ScrolledWindow() sw.set_policy(Gtk.PolicyType.AUTOMATIC, Gtk.PolicyType.ALWAYS) sw.set_shadow_type(Gtk.ShadowType.IN) - sw.add(view) + sw.add(viewbox) model = Gtk.ListStore(object, object, object) self.__fill_list(view, model, value, traceback) view.set_model(model) @@ -236,7 +243,7 @@ def cdf(column, cell, model, iter, data): util.escape(function), line, util.escape(filename))) render = Gtk.CellRendererText() - col = Gtk.TreeViewColumn(text_type(value).replace("_", "__"), render) + col = Gtk.TreeViewColumn(u"", render) col.set_cell_data_func(render, cdf) col.set_visible(True) col.set_expand(True) diff --git a/quodlibet/quodlibet/util/faulthandling.py b/quodlibet/quodlibet/util/faulthandling.py new file mode 100644 index 0000000000..d1af75a055 --- /dev/null +++ b/quodlibet/quodlibet/util/faulthandling.py @@ -0,0 +1,110 @@ +# -*- coding: utf-8 -*- +# Copyright 2016 Christoph Reiter +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 as +# published by the Free Software Foundation + +""" +Interface around faulthandler to save and restore segfault info for the +next program invocation. +""" + +import os +import ctypes +import errno + +try: + import faulthandler +except ImportError: + faulthandler = None + + +_fileobj = None +_enabled = False + + +def enable(path): + """Enable crash reporting and create empty target file + + Args: + path (pathlike): the location of the crash log target path + Raises: + IOError: In case the location is not writable + """ + + global _fileobj, _enabled + + if _fileobj is not None: + raise Exception("already enabled") + + _enabled = True + + if faulthandler is None: + return + + try: + _fileobj = open(path, "rb+") + except IOError as e: + if e.errno == errno.ENOENT: + _fileobj = open(path, "wb+") + + faulthandler.enable(_fileobj) + + +def disable(): + """Disable crash reporting and removes the target file + + Does not raise. + """ + + global _fileobj, _enabled + + assert _enabled + _enabled = False + + if _fileobj is None: + return + + faulthandler.disable() + + try: + _fileobj.close() + os.unlink(_fileobj.name) + except IOError: + pass + _fileobj = None + + +def check_and_clear_error(): + """Returns an error message or None. Calling this will clear the error + so a second call will always return None. + + enable() needs to be called first. + + Returns: + text or None + Raises: + IOError: In case the file couldn't be read + """ + + global _fileobj, _enabled + + assert _enabled + + if _fileobj is None: + return + + _fileobj.seek(0) + text = _fileobj.read().decode("utf-8", "replace").strip() + _fileobj.seek(0) + _fileobj.truncate() + + if text: + return text + + +def crash(): + """Makes the process segfault. For testing purposes""" + + ctypes.string_at(0) diff --git a/quodlibet/tests/helper.py b/quodlibet/tests/helper.py index 3445b8bcdc..6905d0124a 100644 --- a/quodlibet/tests/helper.py +++ b/quodlibet/tests/helper.py @@ -11,6 +11,7 @@ import sys import shutil import locale +import errno from gi.repository import Gtk, Gdk from senf import fsnative, environ @@ -269,7 +270,11 @@ def temp_filename(*args, **kwargs): yield filename - os.remove(filename) + try: + os.remove(filename) + except OSError as e: + if e.errno != errno.ENOENT: + raise def get_temp_copy(path): diff --git a/quodlibet/tests/test_util_faulthandling.py b/quodlibet/tests/test_util_faulthandling.py new file mode 100644 index 0000000000..ff90b25f41 --- /dev/null +++ b/quodlibet/tests/test_util_faulthandling.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Copyright 2016 Christoph Reiter +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 as +# published by the Free Software Foundation + +from tests import TestCase +from .helper import temp_filename + +from quodlibet.util import faulthandling + + +class Tfaulthandling(TestCase): + + def test_basic(self): + with temp_filename() as filename: + faulthandling.enable(filename) + assert faulthandling.check_and_clear_error() is None + faulthandling.disable()