<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/109747/">http://git.reviewboard.kde.org/r/109747/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Found one more issue, I'm afraid.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/109747/diff/2/?file=125736#file125736line832" style="color: black; font-weight: bold; text-decoration: underline;">libnepomukcore/resource/resourcedata.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Nepomuk2::ResourceData::updateIdentifierLists(const QString& oldIdentifier, const QString& newIdentifier)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">786</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">updateKickOffLists</span><span class="p">(</span><span class="n">prop</span><span class="p">.</span><span class="n">uri</span><span class="p">(),</span> <span class="n">oldvalue</span><span class="p">,</span> <span class="n">var</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Don't you need to unlock the data mutex first? You wrote in that method's docu not to call it with m_dataMutex locked (OK, unless the RM mutex is already locked, but I don't see that being the case here).
Same problem in propertyRemoved, in fact.</pre>
</div>
<br />
<p>- David</p>
<br />
<p>On March 28th, 2013, 7:57 p.m. UTC, Simeon Bird 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, David Faure and Vishesh Handa.</div>
<div>By Simeon Bird.</div>
<p style="color: grey;"><i>Updated March 28, 2013, 7:57 p.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;">ResourceWatcher: switch to using KDbusConnectionPool
Tidy up ResourceData::propertyAdded.
If an entry is not found in a QHash, it will return an empty list,
so we only need to check for contains one way.
Rework determineUri so that the RM lock isn't held during the executeQuery
fix lock order in resetAll
early return if nothing to do (noop, just makes the code clearer)
Fix bad threading in ResourceManagerPrivate::addToWatcher dbus usage.
Instead of doing MoveToThread, take the resourceManager mutex before
talking to the ResourceWatcher. This is a lot clearer and less racy.
It should not be too contended now the rm mutex is not held over the
socket communication.
BUG: 305024
FIXED-IN: 4.10.3
ResourceData: Change updateKickOffLists to take three arguments, the uri, old and new values.
This means that it does not need to be under the dataMutex anymore,
which in turn means that we no longer need to remember to lock the RM
mutex before calling it, just to remember to unlock the dataMutex.
This in turn means that we can add a property that isn't NIE::url() or
NAO::identifier to the Resource without taking the mutex at all.
Remove unneeded parameter from ResourceData::resetAll</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Compiled, ran, used activities for a while.</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=305024">305024</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>libnepomukcore/datamanagement/resourcewatcher.cpp <span style="color: grey">(f394ae8705fa882b9f4329f93ea7d18c1cfabc73)</span></li>
<li>libnepomukcore/resource/resource.cpp <span style="color: grey">(7158802cf1e1ba48da86f103aa9311c7acbe24fe)</span></li>
<li>libnepomukcore/resource/resourcedata.h <span style="color: grey">(39508764682b798290c1ae5b74a2384f9cbcbf58)</span></li>
<li>libnepomukcore/resource/resourcedata.cpp <span style="color: grey">(7a979745ca22c88a3a73921d9c0e64b51e064c38)</span></li>
<li>libnepomukcore/resource/resourcemanager.cpp <span style="color: grey">(2b32be01176059932cc1c2cf3b1f348ed91e982b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109747/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>