> 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()) > > Albert Astals Cid wrote: > Please look at the code and tell us where the item deconstructor delists > itself from the list.
I said that it seemed the core issue back then, not that it happens here (for sure) or would be the only trigger. Briefly looking at KArchive, i'd rather bet on a recursion (entries containing a KArchiveEntry being or ultimately pointing down to the same KArchiveDirectory) Just a wild guess, though - but testable if one can reproduce the bug (unless you can assure that cannot be the case) - Thomas ----------------------------------------------------------- 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 > >
