<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/108770/">http://git.reviewboard.kde.org/r/108770/</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;">I don't like this. It's a coverup for a bug that would still be there: if you kill a subjob with Quietly, then it'll be removed from the list of subjobs, but the parent composite job might still be waiting for it, forever.
On hindsight, maybe the default for kill() shouldn't have been Quietly. It only half makes sense for toplevel jobs (the app might not want slotResult to be called when killing a job), and it definitely doesn't make sense for subjobs (the parent job needs to be notified, so Quietly is no good).
In the case of many many subjobs, this would also decrease performance slightly (they already remove themselves in slotResult, but would again attempt to remove themselves upon deletion).
One could argue that if an app calls kill(Quietly) on a subjob then "it knows what it's doing" and it will make sure that the parent job terminates somewhen ... but then it could also call removeSubjob!</pre>
<br />
<p>- David</p>
<br />
<p>On February 4th, 2013, 9:29 a.m. UTC, Kevin Funk 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 kdelibs.</div>
<div>By Kevin Funk.</div>
<p style="color: grey;"><i>Updated Feb. 4, 2013, 9:29 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;">Fix double-free in ~KCompositeJobPrivate
In case a subjob of KCompositeJob has been deleted, this KCompositeJob
instance will crash as soon as it is being destructed, trying to delete
this subjob again.
This fixes a bug in KDevelop.
Backtrace:
1 0x00007ffff7a3f28e in qDeleteAll<QList<KJob*>::const_iterator>
(end=..., begin=...) at /usr/include/qt4/QtCore/qalgorithms.h:322
2 qDeleteAll<QList<KJob*> > (c=QList<KJob *> = {...}) at
/usr/include/qt4/QtCore/qalgorithms.h:330 #3
KCompositeJobPrivate::~KCompositeJobPrivate (this=0x8849850,
__in_chrg=<optimized out>) at ../../kdecore/jobs/kcompositejob.cpp:29
4 0x00007ffff7a3f2c9 in KCompositeJobPrivate::~KCompositeJobPrivate
(this=0x8849850, __in_chrg=<optimized out>) at
../../kdecore/jobs/kcompositejob.cpp:30
5 0x00007ffff7a3fd70 in KJob::~KJob (this=0x880b030, __in_chrg=<optimized out>) at
../../kdecore/jobs/kjob.cpp:73
6 0x00007ffff1a8e5d9 in KDevelop::BuilderJob::~BuilderJob (this=0x880b030, __in_chrg=<optimized
out>) at /home/krf/devel/src/kdevplatform/project/builderjob.cpp:158
BUG: 230692
FIXED-IN: 4.11</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>kdecore/jobs/kcompositejob.h <span style="color: grey">(6ca8eed3ebf8c6f0f5c68d8843bd09a3ea928bbd)</span></li>
<li>kdecore/jobs/kcompositejob.cpp <span style="color: grey">(5ddabd71e5bbb5f0a555a201223a52950b85e786)</span></li>
<li>kdecore/jobs/kcompositejob_p.h <span style="color: grey">(bef06e9bff532b45a8d66380a65117737275be9e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108770/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>