| Summary: | Crash in Sublime::IdealController::raiseView [Sublime::IdealController::raiseView, QObject::setProperty] | ||
|---|---|---|---|
| Product: | [Developer tools] kdevplatform | Reporter: | RJVB <rjvbertin> |
| Component: | sublime | Assignee: | kdevelop-bugs-null |
| Status: | CONFIRMED --- | ||
| Severity: | normal | CC: | |
| Priority: | NOR | ||
| Version: | git master | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| See Also: | https://bugs.kde.org/show_bug.cgi?id=366577 | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: | backtrace | ||
That's a duplicate of bug 366577, no? It's not a check, it's an assert. It denotes that whoever wrote this code originally thought this condition would never happen. Just changing the assert to an if() will work around the problem, but is almost _never_ the correct fix for any issue in kdevelop's code. The problem is that raiseView() is called with a view that is not in m_view_to_action, which shouldn't be the case. To fix the crash, one has to find out how that can happen. I looked around a bit, but didn't see anything obvious... Kevin: judging from the report title they certainly appear to be related. DrKonqi didn't serve up any candidate related tickets though, but that's maybe just another feature that didn't survive the BKO upgrade. Sven: I know it's an assert (which is a form of check...), but I'm not convinced it's a required one in the sense that the unexpected situation cannot be handled any other way than by provoking a crash. If that's the case a version should be used that doesn't disappear in release builds. I have the impression that more than a few developers use this as quick and dirty alternatives to a proper check and error handling when a situation isn't supposed to arise. I've worked with and wrote enough of my own complex code to have accepted the practical fact that you cannot always presume that things will be error proof and work as you intended, esp. when you're dealing with asynchronous events and a human-in-the-loop. You're right that one should try to fix the underlying cause, but that may not be feasible, and in any case that is not an excuse for not handling the situation gracefully if at all possible. Imagine a crash like this occurs when you try to do a patch review, forgot to save and you never get to the proposition to save your changes. That said, if the crash really occurs in m_view_to_action.value() it might also occur in m_view_to_action.contains() if `view` is the culprit here (stale pointer?). I'd expect the return value from m_view_to_action.value() to be undefined when the item isn't in the table, and thus a crash in some unpredictable later statement. Is it possible that `view` is something that should be released through deleteLater() rather than delete? In fact, this *is* a duplicate of 366577. *** Bug 366577 has been marked as a duplicate of this bug. *** Just had another one, I seem to be getting this crash more often these days. Pasting backtrace inline for SEO: Thread 1 (Thread 0x7ff6841ed780 (LWP 8511)): [KCrash Handler] #6 QObject::setProperty (this=this@entry=0x0, name=name@entry=0x7ff67eb4d3b8 "raise", value=...) at /opt/local/var/lnxports/build/_opt_local_site-ports_aqua_qt5-kde-devel/qt5-kde-devel/work/qt-everywhere-opensource-src-5.6.1/qtbase/src/corelib/kernel/qobject.cpp:3839 #7 0x00007ff67eb3c062 in Sublime::IdealController::raiseView (this=<optimized out>, view=view@entry=0x82b6800, mode=Sublime::IdealController::HideOtherViews, mode@entry=Sublime::IdealController::GroupWithOtherViews) at /opt/local/var/lnxports/build/_opt_local_site-ports_kf5_kdevplatform5/kf5-kdevplatform-devel/work/kf5-kdevplatform-5/sublime/idealcontroller.cpp:239 #8 0x00007ff67eb32fd8 in Sublime::MainWindowPrivate::reconstruct (this=0x1dfdd60) at /opt/local/var/lnxports/build/_opt_local_site-ports_kf5_kdevplatform5/kf5-kdevplatform-devel/work/kf5-kdevplatform-5/sublime/mainwindow_p.cpp:446 #9 0x00007ff67eb29f27 in Sublime::MainWindow::setArea (this=this@entry=0x1deca30, area=area@entry=0x1d5f360) at /opt/local/var/lnxports/build/_opt_local_site-ports_kf5_kdevplatform5/kf5-kdevplatform-devel/work/kf5-kdevplatform-5/sublime/mainwindow.cpp:125 #10 0x00007ff67eb37bfb in Sublime::MainWindowOperator::setArea (this=this@entry=0x1dedd40, w=w@entry=0x1deca30, area=area@entry=0x1d5f360) at /opt/local/var/lnxports/build/_opt_local_site-ports_kf5_kdevplatform5/kf5-kdevplatform-devel/work/kf5-kdevplatform-5/sublime/mainwindowoperator.cpp:27 #11 0x00007ff67eb209ae in Sublime::Controller::showAreaInternal (this=this@entry=0x1dedd40, area=area@entry=0x1d5f360, mainWindow=mainWindow@entry=0x1deca30) at /opt/local/var/lnxports/build/_opt_local_site-ports_kf5_kdevplatform5/kf5-kdevplatform-devel/work/kf5-kdevplatform-5/sublime/controller.cpp:133 #12 0x00007ff67eb21598 in Sublime::Controller::showArea (this=0x1dedd40, areaTypeId=..., mainWindow=0x1deca30) at /opt/local/var/lnxports/build/_opt_local_site-ports_kf5_kdevplatform5/kf5-kdevplatform-devel/work/kf5-kdevplatform-5/sublime/controller.cpp:165 #13 0x00007ff63bb683c8 in PatchReviewPlugin::switchToEmptyReviewArea (this=this@entry=0x3b1fd10) at /opt/local/var/lnxports/build/_opt_local_site-ports_kf5_kdevplatform5/kf5-kdevplatform-devel/work/kf5-kdevplatform-5/plugins/patchreview/patchreview.cpp:417 *** Bug 373475 has been marked as a duplicate of this bug. *** I'm now running a local patch that adds `m_view_to_action.contains()` checks before all uses of `m_view_to_action.value()`, and which replaces Q_ASSERT with a qCritical() message. No more crashing, I hope, and if my hunch is correct that these situations occur when a view->action mapping was removed a bit too early it should be an acceptable workaround, too. |
Created attachment 101943 [details] backtrace Application: kdevelop5 (5.0.2) (Compiled from sources) Qt Version: 5.6.1 Frameworks Version: 5.27.0 Operating System: Linux 4.5.7-ck1-mainline-core2-rjvb x86_64 Distribution: Ubuntu 14.04.5 LTS -- Information about the crash: - What I was doing when the application crashed: I hit the "Show differences" action in a changed document's context (right-click) menu. This happened after I had changed the application style from and back to the active style via the style kcm (kcmshell5 style); the active style had been rebuilt and reinstalled with KDevelop running. However I notice that a check of m_view_to_action.contains(view) is made before using .value(view). Evidently these Q_ASSERT checks disappear in production mode. Maybe these could be complemented by a runtime check triggering an early return on failure if that situation is not a guarantee for catastrophic failure down the line but a situation that can indeed be ignored?