Review Request 113965: Possible fix for bug 321100

[Albert Astals Cid]

> On Nov. 20, 2013, 6:02 p.m., Albert Astals Cid wrote:
> > I don't see why this should fix anything. Do you think anyone in the bug can provide a valgrind trace to better understand why it's crashing?
> 
> Christoph Feck wrote:
>     See also https://git.reviewboard.kde.org/r/102981/ which has some discussion and links to related bugs.
> 
> Albert Astals Cid wrote:
>     Why is it related? Who is modifying the entries variable? It's used in 4 places in the file, and actually there's no way to remove stuff from it, so I don't see how it is related to the bug you point.
> 
> Christoph Feck wrote:
>     They are only related because replacing qDeleteAll() with manual deletion fixed the crash for Boudewijn. Since my understanding ended there, I suggested to post a review request.
> 
> Thomas Lübking wrote:
>     See http://lists.kde.org/?t=132194778400005&r=1&w=2
>     Qt 4.8 changed qDeleteAll to rely on the container being immutable.
>     
>     The patch detaches the container, what allows safe operation despite - what's likely happening as it seemed the core issue back than - the container is altered by the deletion of one or more of its items (eg. the items deconstructor delists itself)
>     
>     An alternative solution would be to pass the to-be-deleted objects class a static member flag to skip self delisting and set that around qDeleteAll() (which would be followed by an erase())

Please look at the code and tell us where the item deconstructor delists itself from the list.


- Albert


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


On Nov. 20, 2013, 9:40 a.m., Boudewijn Rempt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113965/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2013, 9:40 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Bugs: 321100
>     http://bugs.kde.org/show_bug.cgi?id=321100
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> While I haven't been able to reproduce the issue on Linux, many Krita users encounter a crash in the destructor of  KArchiveDirectoryPrivate, where all entries are removed with qDeleteAll.
> 
> This patch removes the use of qDeleteAll.
> 
> I'm not 100% sure whether this is correct, but it works for me with the Windows build of kdelibs I use.
> 
> 
> Diffs
> -----
> 
>   kdecore/io/karchive.cpp 88e1de0 
> 
> Diff: http://git.reviewboard.kde.org/r/113965/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Boudewijn Rempt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20131120/0b8603cb/attachment.htm>