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=13219477845r=1w=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.
Thomas Lübking wrote:
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)
Boudewijn Rempt wrote:
I haven't been able to reproduce it myself on Linux. On Windows it was a
lot easier, but there I use an old and completely hacked-up version of
kdelibs. However, when I was giving a workshop at LGM, pretty much half of
the Linux users present (most of them *buntu) users had to disable autosave
to prevent this crash from happening.
I'm puzzled... And I would love a better fix than mine, but that will
have to come from someone who understands karchive -- which I don't, not
really.
Friedrich W. H. Kossebau wrote:
I do not have a better fix yet, but I think I found the root of the
problem:
in case of a duplicated name for an entry in KZip::doPrepareWriting() the
old entry is removed from d-m_fileList, but not from the parentDir directory
holding it:
https://projects.kde.org/projects/kde/kdelibs/repository/revisions/ee5dea835039c8a8f765321378dbed398826f368/entry/kdecore/io/kzip.cpp#L1026
Thus on destruction of that dir the qDeleteAll will try to delete an
entry that is already deleted. Seems that triggers not always a crash,
perhaps because the memory might still be available?
Sadly I do not have a kdelibs development environment setup currently and
also short of disk space, so cannot come up with a patch/unittest. Anyone?
But so far I already see the problem that KArchiveDirectory has a method
addEntry( KArchiveEntry* ) (which currently in case of adding an entry with
the same name qwarns about that, and ignores the new entry), but not some
removeEntry( KArchiveEntry* ). This needs some more thinking what the
proper behaviour should be and how to solve that. Perhaps addEntry(
KArchiveEntry* ) should just overwrite the old entry, that would at least
match the behaviour in KZip::doPrepareWriting()...
Any takers?
Took it myself ;)
Please see review https://git.reviewboard.kde.org/r/114048/ for a proposal
how to fix that problem in KZip::doPrepareWriting().
- Friedrich W. H.
---
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
