Review Request 125220: KService: use a real global static for QThreadStorage rather than Q_GLOBAL_STATIC.

[David Faure]

> On Sept. 14, 2015, 12:11 a.m., Thiago Macieira wrote:
> > Aye, QThreadStorage isn't meant to be used in a Q_GLOBAL_STATIC. I don't understand what you meant by id: QThreadStorageData::get can only be called once the construction has finished, in which case there aren't multiple threads racing.

Yes, the race only exists *when* using Q_GLOBAL_STATIC ;)
In that case the QThreadStorage is created on demand, when there are multiple threads racing.
But your first sentence confirms it then, this change is correct.


- David


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


On Sept. 13, 2015, 10:10 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125220/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 10:10 p.m.)
> 
> 
> Review request for KDE Frameworks, Olivier Goffart and Thiago Macieira.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> Creating QThreadStorage on demand leads to race conditions because the
> id of the storagedata gets assigned once multiple threads are running,
> and read from other threads without locking. I almost submitted a fix for
> Qt when I realized the indirection I had here was unnecessary and wrong.
> 
> QThreadStorage must be created in the main thread, before any thread is running.
> 
> 
> Diffs
> -----
> 
>   src/services/kservicetypefactory.cpp 62fd230b4f7a6558c707d257796c5967f39c3607 
>   src/services/kservicegroupfactory.cpp 08e0bdcd765ab08afdb71cabc640fd21a73f4218 
>   src/services/kservicefactory.cpp 9b0e0c199818fea774c08a4f8fab5aca417927c8 
>   src/services/kmimetypefactory.cpp ba07aa0bc9b5d528454cf426b1feadb049402123 
> 
> Diff: https://git.reviewboard.kde.org/r/125220/diff/
> 
> 
> Testing
> -------
> 
> helgrind ksycocathreadtest no longer complains about a race on QThreadStorageData::id
> 
> 
> Thanks,
> 
> David Faure
> 
>

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