Review Request 129187: Fix dangling pointer in KPackageJob
David Edmundson
david at davidedmundson.co.uk
Fri Oct 14 21:43:28 UTC 2016
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129187/
-----------------------------------------------------------
(Updated Oct. 14, 2016, 9:43 p.m.)
Review request for KDE Frameworks and Plasma.
Changes
-------
rbt-post forgot the commit with the unit test.
Repository: kpackage
Description
-------
A KPackage::Package object uses qexplicitlyshareddata, and it designed
to be kept on the stack and copied. However, PackageJob takes a pointer
to a package, which it later updates, which is expected to exist for the
lifecycle of the job.
This means
Package p = PackageLoader::self()->loadPackage(..);
p.install();
will crash.
Given that, I don't think this is an application error, and but a
library bug.
Both plasmashell installation and uninstallation have this problem:
BUG: 370718
BUG: 369935
As Package is not a QObject we can't just use a QWeakPointer, and
we can't just copy the Package in the packagejob as we need to detatch
and update the *original* KPackage instance. Also to match behaviour we need to do this without changing any other
KPackage instances sharing the same shareddata.
Not a neat fix at all, but there aren't many options that work
without breaking API or behaviour.
Diffs (updated)
-----
autotests/plasmoidpackagetest.h 2488f5c83bd60a693ae15bb5ee5628571ca18252
autotests/plasmoidpackagetest.cpp d7eb9e1a6d7f6e0465627fc0429f2da60c5af238
src/kpackage/package.cpp 207886e2d295773f5239c5804fc7c3a5f1120276
src/kpackage/private/package_p.h c97cbb68fa42b1335699e76e904cea5ebba75eba
src/kpackage/private/packagejob.cpp 84c20b2515ba3f0aba40bf6c1b69829a40bff9ed
Diff: https://git.reviewboard.kde.org/r/129187/diff/
Testing
-------
Crashing unit test
No longer crashes.
Thanks,
David Edmundson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161014/ace51d73/attachment.html>
More information about the Plasma-devel
mailing list