Review Request 129187: Fix dangling pointer in KPackageJob

Anthony Fieroni bvbfan at abv.bg
Sat Oct 15 04:50:07 UTC 2016


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




src/kpackage/package.cpp (line 66)
<https://git.reviewboard.kde.org/r/129187/#comment67174>

    emit?


- Anthony Fieroni


On Окт. 15, 2016, 12:43 преди обяд, David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129187/
> -----------------------------------------------------------
> 
> (Updated Окт. 15, 2016, 12:43 преди обяд)
> 
> 
> 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/15c9f324/attachment.html>


More information about the Plasma-devel mailing list