Review Request 125217: KDirWatch: fix thread safety

David Faure faure at kde.org
Mon Sep 14 07:01:59 UTC 2015



> On Sept. 14, 2015, 1:46 a.m., Michael Pyne wrote:
> > src/lib/io/kdirwatch.cpp, line 1889
> > <https://git.reviewboard.kde.org/r/125217/diff/1/?file=403159#file403159line1889>
> >
> >     The changes themselves seem OK, but if this is really supposed to be thread-safe then shouldn't this cleanup registration also be using an atomic or some other sort of run-once protection? Or is it safe to register the cleanup routine other than once?

Good catch, I'll fix it.


- David


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


On Sept. 13, 2015, 8:18 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125217/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 8:18 p.m.)
> 
> 
> Review request for KDE Frameworks, Albert Astals Cid and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> The use of KDirWatch from KSycoca exposed that KDirWatch was really not
> thread-safe. The internal singleton design brings a ton of races between
> KDirWatch instances. Easy solution: one "singleton" per thread, using
> QThreadStorage.
> 
> The main impact is on memory management; the "singleton" is no longer refcounted
> but simply deleted at the end of the thread, by QThreadStorage (including for
> the main thread). Since this happens at ~QApp time, we need to zero the d pointer
> of KDirWatch::self(), which is deleted by a global-static dtor later on.
> 
> 
> Diffs
> -----
> 
>   src/lib/io/kdirwatch.h f1593e73898c3610c2a560714ed608367120afa8 
>   src/lib/io/kdirwatch.cpp 5e015c10cb99dbe4a21f944d4c0d0b364f2a7263 
>   src/lib/io/kdirwatch_p.h b2ac9d419bb0b0939a4eaa235a55634482db1cc6 
> 
> Diff: https://git.reviewboard.kde.org/r/125217/diff/
> 
> 
> Testing
> -------
> 
> unittests in kcoreaddons still pass, but now kservice's ksycocathreadtest passes every time, unlike before.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150914/94a97b8c/attachment.html>


More information about the Kde-frameworks-devel mailing list