Review Request 129187: Fix dangling pointer in KPackageJob

[Marco Martin]

> On Oct. 15, 2016, 4:50 a.m., Anthony Fieroni wrote:
> > src/kpackage/package.cpp, line 66
> > <https://git.reviewboard.kde.org/r/129187/diff/2/?file=482226#file482226line66>
> >
> >     emit?

ah yes, please :)


- Marco


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


On Oct. 14, 2016, 9:43 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> 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/plasma-devel/attachments/20161015/57301712/attachment.html>