Review Request 129187: Fix dangling pointer in KPackageJob

David Edmundson david at davidedmundson.co.uk
Sun Oct 16 12:13:17 UTC 2016


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129187/
-----------------------------------------------------------

(Updated Oct. 16, 2016, 12:13 p.m.)


Status
------

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
-------

Submitted with commit cfb69e21fb0aad92f403487b4c8a75b2b0bc8041 by David Edmundson to branch master.


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
-----

  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/20161016/72267809/attachment.html>


More information about the Kde-frameworks-devel mailing list