Review Request 118739: Make KSharedConfig thread-safe

[Matthew Dawson]

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


LGTM.  Just a couple of minor points, but if you prefer to get this is in now, so that kio can be thread safe these can be cleaned up after the fact.


src/core/ksharedconfig.cpp
<https://git.reviewboard.kde.org/r/118739/#comment42037>

    Should the KSharedConfig not all be synced to disk before its thread is destroyed?  Reading QThreadStorage's documentation, the sync can probably just happen in the deconstructor, as long as the thread isn't the main thread (or maybe just that qApp is still valid).



src/core/ksharedconfig.cpp
<https://git.reviewboard.kde.org/r/118739/#comment42036>

    Why create a template function here, when the T must be GlobalSharedConfigList?  Why not just put the logic in globalSharedConfigList?



src/core/ksharedconfig.cpp
<https://git.reviewboard.kde.org/r/118739/#comment42038>

    Can you also please document the new behaviour of openConfig, that each instance is thread specific?


- Matthew Dawson


On June 14, 2014, 5:20 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118739/
> -----------------------------------------------------------
> 
> (Updated June 14, 2014, 5:20 a.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> Make KSharedConfig thread-safe
> 
> ... by having a different list of shareable objects per thread.
> 
> 
> Diffs
> -----
> 
>   src/core/ksharedconfig.cpp f4b4c766fe19fac92a0651ecc55a89ec3b7636b0 
> 
> Diff: https://git.reviewboard.kde.org/r/118739/diff/
> 
> 
> Testing
> -------
> 
> helgrind kiocore-threadtest (not committed yet)
> 
> no races detected in KSharedConfig anymore.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140617/7dc1e787/attachment.html>