Summary: | Ark crashes in KUiServerV2JobTracker::registerJob() when turning on second monitor | ||
---|---|---|---|
Product: | [Frameworks and Libraries] frameworks-kjobwidgets | Reporter: | hbr <nepnep> |
Component: | general | Assignee: | kdelibs bugs <kdelibs-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | crash | CC: | aacid, andrea, cullmann, dabiswas112, hsushipei1, jdanek, justin.zobel, kde, mpyne, nate, rthomsen6, toralf.foerster, torokati44 |
Priority: | HI | Keywords: | drkonqi, wayland |
Version: | 5.91.0 | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | https://invent.kde.org/frameworks/kjobwidgets/commit/5aeba3f01ef8cdf723813cacdd29945328288663 | Version Fixed In: | 5.97 |
Attachments: |
kcrash-metadata kded5
kcrash-metadata plasmashell kcrash-metadata org_kde_powerdevil |
Description
hbr
2022-02-15 17:39:04 UTC
Turns out it is not just Ark crashing when I do this but the entire plasmashell. It is reproducable and happens every time. This did not happen with KDE Plasma 5.23.X on Wayland and does not happen with KDE Plasma 5.24.X on X11. However I don't seem to be able to get stack traces since drkonqi / the crash reporter also crashes shortly after / gets closed when plasmashell gets restarted. Is there any way I can trigger the stack trace collection manually after plasmashell has been retarted? Created attachment 147055 [details]
kcrash-metadata kded5
Created attachment 147056 [details]
kcrash-metadata plasmashell
Created attachment 147057 [details]
kcrash-metadata org_kde_powerdevil
Tried again today with Plasma 5.24.2 and it still happens. However drkonqi did not start so I could not create a trace. I only got the ini files for kcrash from "~/.cache" (attached). No idea if they are of any news. Updated to Plasma 5.24.2 -> rebooted -> logged into the Wayland session -> turned on the second monitor -> plasmashell crashed and it took about 30 seconds until it was restarted -> drkonqi did not show up #7 0x00007f47359d47b5 in KUiServerV2JobTracker::registerJob (this=0x555e082d4200, job=<optimized out>) at /usr/src/debug/kjobwidgets-5.91.0/src/kuiserverv2jobtracker.cpp:187 Can you file a new bug report for the Plasma crash, and attach a backtrace of Plasma crashing? Thanks! I have since switched some hardware and am now running Fedora 36 pre-release. The crash still happens there but in a much more graceful way (e.g. the restart of Plasma Shell is not delayed by about 30s + but pretty much happens instantly). I will check and report again once Fedora 36 is properly released, so this can be closed. I unfortunately cannot reproduce the Ark crash when restarting plasma while extracting a file. The crash could occur if Plasma restarts, thus our jobtracker re-registers the job, but the job is already gone or something. But I failed to reproduce it by extracting a file or manually using kjobcreator. *** Bug 452808 has been marked as a duplicate of this bug. *** *** Bug 451474 has been marked as a duplicate of this bug. *** *** Bug 451855 has been marked as a duplicate of this bug. *** *** Bug 452167 has been marked as a duplicate of this bug. *** *** Bug 453377 has been marked as a duplicate of this bug. *** I collected all other bugs with the same backtrace now here. *** Bug 456175 has been marked as a duplicate of this bug. *** *** Bug 455696 has been marked as a duplicate of this bug. *** *** Bug 455658 has been marked as a duplicate of this bug. *** Seems this crash is not that uncommon :( I suspect the issue is in using a deleted KJob when re-registering a job view (kuiserverv2jobtracker.cpp:197) // Watch the server registering/unregistering and re-register the jobs as needed if (!d->serverRegisteredConnection) { d->serverRegisteredConnection = connect(serverProxy(), &KSharedUiServerV2Proxy::serverRegistered, this, [this]() { const auto staleViews = d->jobViews; // elided for (auto it = staleViews.begin(), end = staleViews.end(); it != end; ++it) { KJob *job = it.key(); const JobView &view = it.value(); // elided registerJob(job); In the section of code right after this in registerJob, the code is careful to ensure that a QPointer<KJob> is used in order to verify that the KJob being registered won't have been deleted in the functor to be added to the event loop by QTimer (kuiserverv2jobtracker.cpp:248): QPointer<KJob> jobGuard = job; QTimer *delayTimer = new QTimer(); delayTimer->setSingleShot(true); connect(delayTimer, &QTimer::timeout, this, [this, job, jobGuard, desktopEntry] { // elided }); This part of the code also references the d->jobViews the re-registration code uses earlier, so if it were possible to assume that a KJob* held in d->jobViews was enough to guarantee the KJob were still valid, the later delay timer could have done the same thing. It would take more code to do this so that's not airtight evidence, but I can't find anywhere else that guarantees a KJob ending also removes its JobView (the re-registration code as much as calls it a "stale" job view...) so I suspect this is the cause of this crash. I'll put together a patch that uses the same QPointer<KJob> jobGuard trick and open an MR. Sounds like a reasonable explanation. (In reply to Christoph Cullmann from comment #20) > Sounds like a reasonable explanation. well, maybe. I did find the code meant to update the jobView structure when the KJob is finished, but it is a) called indirectly (from KUiServerV2JobTracker::unregisterJob), and b) has a weird special case to handle terminated KJobs that don't have an existing view (presumably because the signal that would create it being later in the event loop): // Remember that the job finished in the meantime and // terminate the JobView once it arrives d->scheduleUpdate(job, QStringLiteral("terminated"), true); if (job->error()) { d->scheduleUpdate(job, QStringLiteral("errorCode"), static_cast<uint>(job->error())); d->scheduleUpdate(job, QStringLiteral("errorMessage"), job->errorText()); } But scheduleUpdate() simply recreates the jobView mapping to the soon-to-be dead KJob and if the re-registration happens before the "terminate the JobView once it arrives" occurs we're in for a bad time, as the re-registration code assumes all entries in the map of jobs to jobViews have valid KJobs. I'll take a look at how best to handle the weird lifetimes here and try to open the MR tonight. Git commit b8752085d2a480dfc93d2d422c463a59d46af5ee by Michael Pyne. Committed on 02/07/2022 at 22:13. Pushed by mpyne into branch 'work-bug-450325-fix-crash'. ui-server: Fix crash by only re-registering live KJobs. This addresses a frequently-reported crash in the job tracker for KUiServerV2 that occurs when attempting to re-register new job views for active KJobs after a new UI server comes online. Although I have not been able to reproduce the crash myself, (by attempting to use both long-lived and short-lived file transfers from Dolphin and restarting plasmashell), inspection of the code shows that it is possible for there to be deleted KJobs pointing to JobView objects during some portions of the job tracker's lifetime. The current code deals with this in situations including DBus calls to create a U/I view for a KJob (the KJob may terminate before the DBus reply is received) and even a short delay that can be optionally introduced (the KJob may terminate before the delay elapses). A QPointer<KJob> is used as a guard in these situations, but there is no similar guard for the re-registration code. In this case we cannot use QPointer<KJob> to guard the job's lifetime because the KJob must be alive when the QPointer<KJob> is created, and this crash occurs when the KJob is terminated. However the KJob's destruction should lead to the unregisterJob() function being called, which handles removing the terminated KJob from the map of job views with only one exception, where instead the job view for the KJob has its "terminated" pending status set. So the fix here checks for the "terminated" state in the same way as performed in requestView(), and if the KJob is terminated, handles requesting the job view to terminate the U/I and finally removing the terminated KJob from the map of job views. By doing this, we avoid passing a deleted KJob to the registerJob() function, which will attempt to dereference it and crash the application. M +16 -4 src/kuiserverv2jobtracker.cpp https://invent.kde.org/frameworks/kjobwidgets/commit/b8752085d2a480dfc93d2d422c463a59d46af5ee I thought that a MR needed to be merged to master to have its BUG: close a bug, reopening until a fix is merged. A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kjobwidgets/-/merge_requests/22 I think the branch must be names work/.... to avoid the hooks. That's correct, yeah. It's also requires for other people to be able to rebase your MRs. Git commit 5aeba3f01ef8cdf723813cacdd29945328288663 by Michael Pyne. Committed on 09/07/2022 at 18:42. Pushed by mpyne into branch 'master'. ui-server: Fix crash by only re-registering live KJobs. This addresses a frequently-reported crash in the job tracker for KUiServerV2 that occurs when attempting to re-register new job views for active KJobs after a new UI server comes online. Although I have not been able to reproduce the crash myself, (by attempting to use both long-lived and short-lived file transfers from Dolphin and restarting plasmashell), inspection of the code shows that it is possible for there to be deleted KJobs pointing to JobView objects during some portions of the job tracker's lifetime. The current code deals with this in situations including DBus calls to create a U/I view for a KJob (the KJob may terminate before the DBus reply is received) and even a short delay that can be optionally introduced (the KJob may terminate before the delay elapses). A QPointer<KJob> is used as a guard in these situations, but there is no similar guard for the re-registration code. In this case we cannot use QPointer<KJob> to guard the job's lifetime because the KJob must be alive when the QPointer<KJob> is created, and this crash occurs when the KJob is terminated. However the KJob's destruction should lead to the unregisterJob() function being called, which handles removing the terminated KJob from the map of job views with only one exception, where instead the job view for the KJob has its "terminated" pending status set. So the fix here checks for the "terminated" state in the same way as performed in requestView(), and if the KJob is terminated, handles requesting the job view to terminate the U/I and finally removing the terminated KJob from the map of job views. By doing this, we avoid passing a deleted KJob to the registerJob() function, which will attempt to dereference it and crash the application. See also merge request !22 M +16 -4 src/kuiserverv2jobtracker.cpp https://invent.kde.org/frameworks/kjobwidgets/commit/5aeba3f01ef8cdf723813cacdd29945328288663 |