<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/105367/">http://git.reviewboard.kde.org/r/105367/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 2nd, 2012, 7:50 p.m., <b>Laszlo Papp</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;">I would prefer if you did not submit this. First, why are you trying to work on an unrelated method? Backtraces write about the abort method. Second, I would not like to get anything committed in a rush to a stable module without clear logic why the crash is happening, and why the fix would address that.
I am afraid, this might require more investigation from your side.</pre>
</blockquote>
<p>On July 2nd, 2012, 8:21 p.m., <b>Mark Gaiser</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;">Sorry for not mentioning that.
Please don't start with "why are you trying to work on an unrelated method?" ... I took the time to backtrace it, found out that abort itself isn't doing anything that is (or seems) wrong so i searched deeper in that class to see what could cause abort to crash.
The crash is happening because the d pointer is gone and abort tries to call an object on the d pointer. Yes, you could argue that it needs to check for d as well, but i think that's wrong because the GHNS list (for wallpapers as an example) is working fine the first time, but crashing the second time like i described in #251871 (last comment by my with the steps) thus something else must have been wrong. That d pointer is gone because deleteLayer(); is called in void BaseJob::dataFinished() which removes it's current object http://qt-project.org/doc/qt-4.8/qobject.html#deleteLater thus you get a crash in the abort function because d->m_reply simply doesn't exist because d is gone.
I'm not quite sure anymore why i removed "d->m_reply->deleteLater();" as well.</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;">Your problem is basically that, knewstuff has been trying to abort a non-running process without even having any smart pointer or destroyed signal handling underneath. This causes that the pointer has been tried to be deleted twice. The second try is essentially the deletion of a dangling pointer, which is not good.
Well, to be honest, abort() should be an abort, and not deletion in any circumstances. Means, it would abort the running job (not finished). Abort now seems more like a delayed deletion which is a suboptimal concept in my opinion. I would personally change the behavior in this case for good which does not mean API or ABI break. Once, the users complain (if any), then it can be reverted like the QUrl change in Qt 4.8.
Also, if you take a look at the KJob implementation, they also have a hack for the auto/manual deletion for finished. Again, IMO, the definition of "abort" is different to me, what is now in there. A finished job is not a running job, so nothing to abort.</pre>
<br />
<p>- Laszlo</p>
<br />
<p>On June 26th, 2012, 9:04 p.m., Mark Gaiser wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Attica.</div>
<div>By Mark Gaiser.</div>
<p style="color: grey;"><i>Updated June 26, 2012, 9:04 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;">Get hot new stuff could easily crash when filtering the retrieved data. The attached bug report shows a lot of backtraces that gave me a nice starting point in fixing this. I'm not quite sure if i fixed it the proper way. Somehow the original code was emitting the current class pointer followed by removing the same class. I simply removed the remove lines (deleteLayer) since the abort function is also cleaning the stuff up. I hope i'm not introducing a memory leak now?</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;">Tested it out with the descriptions in the provided bug. It seems to be working just fine now. No more crashes.</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=251871">251871</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>lib/atticabasejob.cpp <span style="color: grey">(c4f0bc8)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105367/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>