| Summary: | Crash when closing okular | ||
|---|---|---|---|
| Product: | [Applications] okular | Reporter: | Riccardo Escher <riccardoescher> |
| Component: | general | Assignee: | Okular developers <okular-devel> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | bugs.kde.org, haxtibal, mail, marcelo.jimenez, oss+kde+bugzilla, ZigZag.g.h |
| Priority: | NOR | Keywords: | drkonqi |
| Version: | 1.4.2 | ||
| Target Milestone: | --- | ||
| Platform: | openSUSE | ||
| OS: | Linux | ||
| See Also: | https://bugs.kde.org/show_bug.cgi?id=394534 | ||
| Latest Commit: | https://commits.kde.org/okular/d1ea28fc7338c24910faff77c6308a3e4f32369a | Version Fixed In: | 18.12.0 |
| Sentry Crash Report: | |||
|
Description
Riccardo Escher
2018-06-22 19:25:12 UTC
*** Bug 394707 has been marked as a duplicate of this bug. *** *** Bug 394429 has been marked as a duplicate of this bug. *** *** Bug 395684 has been marked as a duplicate of this bug. *** *** Bug 399207 has been marked as a duplicate of this bug. *** *** Bug 396783 has been marked as a duplicate of this bug. *** *** Bug 397737 has been marked as a duplicate of this bug. *** Just reproduced this on openSUSE Tumbleweed, will have a closer look next days. It never happened with okular master in my Ubuntu environment before. Obvious differences are newer versions of Qt5 and kconfig in the Tumbleweed configuration. Quite interesting. The crash is caused by a dangling KConfig* pointer inside the KConfigGroup that's passed to Shell::readProperties. The KConfig was deleted preliminary during Shell::openUrl(). There a QDialog event loop processed a XSMP "SaveYourself" message, and the related handler code eventually deleted a globally used KConfig instance.
It goes like this
1. KMainWindow::restore creates a KConfig *config = KConfigGui::sessionConfig(). This KConfig is managed singleton-like, see static KConfig *KConfigGui::s_sessionConfig.
2. KMainWindow::readPropertiesInternal constructs a KConfigGroup grp(config, ...) with that KConfig, and passes it on to Shell::readProperties (polymorphic call from framework to application).
3. The trouble begins in openUrl(). There we can't find a document, and spawn a QDialog for the "file not found" message. QDialog has an event loop that's stacked onto the application event loop.
It happens that ksmserver sent us an XSMP "SaveYourself" message (that's expected after client registration), which now gets processed by the QDialog event loop. We handle "SaveYourself" with KMWSessionManager::saveState => KConfigGui::setSessionConfig and hit the code
if (hasSessionConfig()) { delete s_sessionConfig; s_sessionConfig = nullptr; }
Sadly, the old s_sessionConfig address is still stored in the KConfigGroup, where the pointer is dangling now.
4. Inside group.readEntry<int>( SESSION_TAB_KEY, 0 ) the dangling KConfig* is accessed, resulting in segfault.
I'm not yet sure how to fix it. Shall we discuss it with some KF5 guys? It seems a bit fragile if a framework passes some wrapped up pointer to application code, and said pointer can become invalid for non-obvious reasons.
Git commit d1ea28fc7338c24910faff77c6308a3e4f32369a by Tobias Deiminger. Committed on 29/10/2018 at 20:44. Pushed by tobiasdeiminger into branch 'master'. Avoid crash during session restore Summary: Do all access to the passed KConfigGroup really synchronous to KMainWindow::readProperties, then we're safe. Currently kxmlgui can't guarantee that the passed KConfigGroup is still valid after our call to Shell::openUrl(). This is because inside Shell::openUrl, QDialog::exec may get called. The stacked event loop processes all kinds of asynchronous events, and litterally *anything* can happen. E.g. incoming ICE and DBus messages may be processed. In bug 395765 it happened that XSMP SafeYourself was processed, which calls KConfigGui::setSessionConfig, which leaves the KConfig pointer inside KConfigGroup dangling. Test Plan: - get recent Qt5 and KF5 - manually save desktop session while a document is open in okular - modify ~/.config/session/okular_<sessionid> so that Urls points to non existing file - manually restore session with okular -session <session_id>, dialog will open and warn about non existent file - okular shall not crash after closing that dialog Reviewers: aacid Reviewed By: aacid Subscribers: aacid, okular-devel Tags: #okular Differential Revision: https://phabricator.kde.org/D16457 M +1 -1 shell/shell.cpp https://commits.kde.org/okular/d1ea28fc7338c24910faff77c6308a3e4f32369a Wow! Thank you very much! I'm constantly being hit by this in 1.5.3 (18.08.3). In what version is this fixed? |