<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/128113/">https://git.reviewboard.kde.org/r/128113/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Юни 16th, 2016, 11:49 преди обяд EEST, <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;">Looking at https://git.reviewboard.kde.org/r/128150 (to basically read the libkerfuffle code), I see what the problem is.
You're mixing KJob and QThread, which is only fine if none of the KJob API is called from the secondary thread. I.e. the KJob should only sit on top of the thread and report its progress (e.g. via signals emitted by the thread). This is almost what you're doing, except for this:
connect(archiveInterface(), &ReadOnlyArchiveInterface::finished, this, &Job::onFinished, Qt::DirectConnection);
Due to the DirectConnection, this will call onFinished, and therefore KJob::emitResult, from the secondary thread. This leads to race conditions inside KJob, and it leads to deletion of the job happening in the wrong thread.
This should definitely not be a direct connection.
This would also fix the race on m_isRunning that your code currently has (read/written in main thread (isRunning() and start()), but set to true in secondary thread in emitResult()).
You should use helgrind (http://www.kdab.com/~dfaure/helgrind.html) on that code to make sure there are no other race conditions.
Conclusion from this analysis: I don't think there's a bug in kjobwidgets, until proven otherwise (preferably with an actual test or unittest that leads to a crash).</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">unregisterJob cannot be called more than ones or it trys to disconect self more than ones. So Qt must be smart enough, but it shouldn't. Fix is pretty easy, check must be <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">bofore</em> parent call and job must be removed from list.
if (!d->progressJobView.contains(job))
...
KJobTrackerInterface::unregisterJob(job);</p></pre>
<br />
<p>- Anthony</p>
<br />
<p>On Юни 10th, 2016, 8:07 преди обяд EEST, Anthony Fieroni wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks and David Faure.</div>
<div>By Anthony Fieroni.</div>
<p style="color: grey;"><i>Updated Юни 10, 2016, 8:07 преди обяд</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kjobwidgets
</div>
<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;"><ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Fix memory leak in finished</li>
</ul></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">ark --changetofirstpath --add --autofilename zip kjobwidgets
Crash before, fix with patch</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/kuiserverjobtracker.cpp <span style="color: grey">(ebed3a5)</span></li>
<li>tests/kjobtrackerstest.cpp <span style="color: grey">(7ef9e07)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128113/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/5b87387e-7e7b-4982-b91b-a18f72414509__backtrace">backtrace</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/07/ba4b0150-5e01-4cc1-8776-7530d053d6f0__memcheck">memcheck</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/07/f8813ccc-8835-4255-842c-29c57c2dea23__memcheck2">memcheck 7 errorrs</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>