<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112011/">http://git.reviewboard.kde.org/r/112011/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 16th, 2013, 8:23 a.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"Testing done" is empty. Please make sure you test this, because it could change the thread in which registered objects get called (I'm not actually sure how that works with QDBusConnection).
For outgoing calls using KDBusConnectionPool is a very simple change, but for registering objects for incoming calls, better double-check.
"Ship it" if you tested that the incoming calls happen in the right thread.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks for the review.
I've been testing this for over a week and it's fine as long as all the object registrations are done in the same thread using the same QDBusConnection. Otherwise, we have ugly problems.
Right now, this patch cannot be shipped cause the architecture of Nepomuk makes each service automatically register its name and some other properties using QDBusConnection::sessionBus() instead of KDBusConnection (rightly so), and therefore we have a situation where -
$ qdbus org.kde.NepomukStorage and $ qdbus org.kde.nepomuk.services.nepomukstorage give different outputs even though both of them belong to the same process.
I guess it will be better to only use KDbusConnectionPool everywhere but the main thread.</pre>
<br />
<p>- Vishesh</p>
<br />
<p>On August 11th, 2013, 7:38 a.m. UTC, Vishesh Handa wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Nepomuk and David Faure.</div>
<div>By Vishesh Handa.</div>
<p style="color: grey;"><i>Updated Aug. 11, 2013, 7:38 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"> Use KDBusConnectionPool instead of QDBusConnection
QDBusConnection cannot be used across multiple threads
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=315078">315078</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=319165">319165</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>services/storage/backup/backupmanager.cpp <span style="color: grey">(60ce815)</span></li>
<li>services/storage/datamanagementcommand.cpp <span style="color: grey">(5d8036c)</span></li>
<li>services/storage/ontologyloader.cpp <span style="color: grey">(9acac5f)</span></li>
<li>services/storage/query/folderconnection.cpp <span style="color: grey">(1731180)</span></li>
<li>services/storage/query/queryservice.cpp <span style="color: grey">(593abd9)</span></li>
<li>services/storage/repository.cpp <span style="color: grey">(6a2dd18)</span></li>
<li>services/storage/resourcewatcherconnection.cpp <span style="color: grey">(0b245d7)</span></li>
<li>services/storage/resourcewatchermanager.cpp <span style="color: grey">(0b20cb8)</span></li>
<li>services/storage/storage.cpp <span style="color: grey">(1b63dff)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112011/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>