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/kde-frameworks-devel/attachments/20161014/ace51d73/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list