Review Request 108770: Fix double-free in ~KCompositeJobPrivate

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108770/#review26689
-----------------------------------------------------------


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!

- David Faure


On Feb. 4, 2013, 9:29 a.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108770/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2013, 9:29 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   kdecore/jobs/kcompositejob.h 6ca8eed3ebf8c6f0f5c68d8843bd09a3ea928bbd 
>   kdecore/jobs/kcompositejob.cpp 5ddabd71e5bbb5f0a555a201223a52950b85e786 
>   kdecore/jobs/kcompositejob_p.h bef06e9bff532b45a8d66380a65117737275be9e 
> 
> Diff: http://git.reviewboard.kde.org/r/108770/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130205/912a639f/attachment.htm>