Review Request 118614: Make KIO thread-safe

[Kevin Ottens]

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

Ship it!


Would have never guessed it required so little adjustments. That's great!
Definitely a go IMO.

As for KDBusConnectionPool it's fine to use it I think. As you pointed out it'll be easy to switch later on if QTBUG-39528 gets addressed.

- Kevin Ottens


On June 8, 2014, 10:11 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118614/
> -----------------------------------------------------------
> 
> (Updated June 8, 2014, 10:11 a.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Thiago Macieira.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> Unittest for kiocore's thread-safety
> 
> 
> KIO thread safety: use one DBus connection per thread
> 
> 
> KIO thread safety: don't use static buffers.
> 
> They obviously lead to data races between threads.
> 
> KIO: create one instance of Scheduler per thread.
> 
> 
> Diffs
> -----
> 
>   src/core/scheduler.cpp e762e369b8d74971831dc1e9f4d083bfd77b8e30 
>   src/core/connectionbackend.cpp 532141af5ab7676d1287379e56ff48bd7fa10c37 
>   autotests/threadtest.cpp PRE-CREATION 
>   autotests/CMakeLists.txt 81d261b8620e0a0512a5bc021ed189e910c0922a 
> 
> Diff: https://git.reviewboard.kde.org/r/118614/diff/
> 
> 
> Testing
> -------
> 
> helgrind'ing the unittest.
> 
> I found and fixed many races within Qt by doing that...
> Race on QMutex::id --> https://codereview.qt-project.org/86237
> Race in QThreadStorage --> https://codereview.qt-project.org/87047
> Race in QLoggingCategory --> https://codereview.qt-project.org/87054
> 
> Also reported QTBUG-39528 (QDBusConnection::sessionBus race)  (but we have KDBusConnectionPool for that)
> 
> And there are still more races in Qt: 
> QMutex::isRecursive, qdbus_loadLibDBus, QDBusConnectionPrivate::connectSignal, qt_message_print....
> 
> And a few in our code:
> the getenv(KDE_FORK_SLAVES) in KIO, and the dbus interface for klauncher in KDEInitInterface::ensureKdeinitRunning.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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