| Summary: | Crash in libksysguard process reading in TM update | ||
|---|---|---|---|
| Product: | [Plasma] plasmashell | Reporter: | David Edmundson <kde> |
| Component: | Task Manager and Icons-Only Task Manager | Assignee: | Eike Hein <hein> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | ahiemstra, kde, nate, plasma-bugs |
| Priority: | NOR | ||
| Version: | master | ||
| Target Milestone: | 1.0 | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | https://invent.kde.org/plasma/libksysguard/commit/2d522614357d6dc5a5a7b738a35574f5d8f69da5 | Version Fixed In: | 5.20.2 |
| Sentry Crash Report: | |||
|
Description
David Edmundson
2020-10-23 22:59:27 UTC
I think the crash is as follows:
connect(runnable, &ReadProcSmapsRunnable::finished, this, [this, pid, runnable]() {
Q_EMIT processUpdated(pid, { { Process::VmPSS, runnable->pss() } });
runnable->deleteLater();
}, Qt::DirectConnection);
We set the context to "this" so we should auto disconnect when the Processes object is destroyed...
*BUT*
Qt::DirectConnection
means that this is run in the runner thread
so "this" can be valid at the time of the signal being emitted and null by the time we get to this emit.
Currently reproducible whilst by having Hydrogen from flatpak open. (flatpak is significant as it's fetching PID2 like an idiot...) Debug shows TM fetches PID data like crazy, it also seems to not have a singleton Processes object backing things. My cgroup TM patch might suppress this? but we should fix the root cause anyway and also backport something. Simple fix: https://phabricator.kde.org/P642 A possibly relevant merge request was started @ https://invent.kde.org/plasma/libksysguard/-/merge_requests/90 Git commit 9b5161cb9a16eaad205631b2a4c382dbbbe2072c by David Edmundson. Committed on 25/10/2020 at 23:00. Pushed by davidedmundson into branch 'master'. Handle smap read result in the correct thread We set the context to "this" so we should auto disconnect when the Processes object is destroyed. But Qt::DirectConnection means that is run in the runner thread. It is possible to have a scenario where "this" is active when the connection is made, but destroyed before we get to this line. Related: bug 428048 But scoping the connect to Processes existing we would have leaked the worker if Processes got destroyed whilst the worker was running. M +8 -5 processcore/processes_linux_p.cpp M +3 -8 processcore/read_procsmaps_runnable.cpp M +1 -4 processcore/read_procsmaps_runnable.h https://invent.kde.org/plasma/libksysguard/commit/9b5161cb9a16eaad205631b2a4c382dbbbe2072c Git commit 2d522614357d6dc5a5a7b738a35574f5d8f69da5 by David Edmundson. Committed on 26/10/2020 at 17:25. Pushed by davidedmundson into branch 'Plasma/5.20'. Handle smap read result in the correct thread We set the context to "this" so we should auto disconnect when the Processes object is destroyed. But Qt::DirectConnection means that is run in the runner thread. It is possible to have a scenario where "this" is active when the connection is made, but destroyed before we get to this line. Related: bug 428048 But scoping the connect to Processes existing we would have leaked the worker if Processes got destroyed whilst the worker was running. (cherry picked from commit 9b5161cb9a16eaad205631b2a4c382dbbbe2072c) M +8 -5 processcore/processes_linux_p.cpp M +3 -8 processcore/read_procsmaps_runnable.cpp M +1 -4 processcore/read_procsmaps_runnable.h https://invent.kde.org/plasma/libksysguard/commit/2d522614357d6dc5a5a7b738a35574f5d8f69da5 |