Review Request 127795: [DataEngine] Fix memory leak and possible crash + detailed test

Anthony Fieroni bvbfan at abv.bg
Sun May 1 05:53:09 UTC 2016



> On Май 1, 2016, 3:14 преди обяд, Michael Pyne wrote:
> > src/plasma/private/dataenginemanager.cpp, line 49
> > <https://git.reviewboard.kde.org/r/127795/diff/1/?file=461952#file461952line49>
> >
> >     Is it safe to remove this, given that even the change  to DataEngineManager::loadEngine to ->ref all returned engines still doesn't ->ref the null engine?

NullEngine is not to be managed or refered, it stays alive with DataEngineManagerPrivate.


> On Май 1, 2016, 3:14 преди обяд, Michael Pyne wrote:
> > src/plasma/datacontainer.cpp, line 170
> > <https://git.reviewboard.kde.org/r/127795/diff/1/?file=461950#file461950line170>
> >
> >     This and the next 3 Qt::UniqueConnection flags appear to be unneeded: by this point in the code path there have already been checks for these signal-slot connections and what appear to be correct calls to QObject::disconnect().

So relay->receiverCount() == 1 it's not disconnected


> On Май 1, 2016, 3:14 преди обяд, Michael Pyne wrote:
> > src/plasma/dataengine.cpp, line 416
> > <https://git.reviewboard.kde.org/r/127795/diff/1/?file=461951#file461951line416>
> >
> >     This should be 1 instead of 0 given the changes you've made elsewhere (unless the existing value is a bug...).
> >     
> >     But, the entire set of changes to reference count handling appear to be unnecessary. Why the change?

You are right from one point of view, isValid must not be 'true' i will correct it. Since this maybe nullEngine is wrong to be 1 or -1. One engine that is not refered must be invalid and useless. Manager must decide.


- Anthony


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


On Април 30, 2016, 6:18 след обяд, Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127795/
> -----------------------------------------------------------
> 
> (Updated Април 30, 2016, 6:18 след обяд)
> 
> 
> Review request for KDE Frameworks and Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> ^^
> 
> 
> Diffs
> -----
> 
>   autotests/pluginloadertest.h 61fc963 
>   autotests/pluginloadertest.cpp 4f96780 
>   src/plasma/datacontainer.cpp ee067db 
>   src/plasma/dataengine.cpp f942926 
>   src/plasma/private/dataenginemanager.cpp 08e42fb 
> 
> Diff: https://git.reviewboard.kde.org/r/127795/diff/
> 
> 
> Testing
> -------
> 
> Test pass.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160501/1c5c8698/attachment.html>


More information about the Kde-frameworks-devel mailing list