Hi, 2013/8/6 Frank Reininghaus: > Hi, > > 2013/8/6 KDE CI System <n...@kde.org>: >> See <http://build.kde.org/job/kdelibs_frameworks_qt5/982/changes> > > the crash in KDirListerTest has been caused by my recent change > > https://git.reviewboard.kde.org/r/111789/ > > First of all, I apologize for not having run that test before. I had > thought that running KFileItemTest is sufficient because I had assumed > that it is impossible (or at least extremely unlikely) that anyone > relied on the low-level memory layout of a QList<KFileItem>. But I was > wrong :-( > > The problem is that "void KCoreDirListerCache::slotUpdateResult( KJob > * j )" has a local variable > > QHash<QString, KFileItem*> fileItems > > where pointers to the KFileItems in the KFileItemList dir->lstItems > are stored. The function also appends items to dir->lstItems, such > that all pointers in "fileItems" become invalid when a reallocation of > memory is needed. > > Moreover, KDirListerCache has a member > > QSet<KFileItem*> pendingRemoteUpdates > > which might also be problematic (I haven't checked that in detail yet though). > > @David: do you know what the reason for storing these pointers, > instead of the KFileItems themselves, is? The only advantage I could > imagine is that this saves updates for the atomic reference counter. > I'm not sure though if that causes more overhead than having a pointer > to the KFileItem, which is in turn a pointer to KFileItemPrivate (and > the overhead that the memory layout of KFileItemList, that > KDirListerCache relies on, causes because the list also stores > pointers to the KFileItems). > > IMHO, it looks like using KFileItems, instead of pointers to them, in > KDirListerCache might be the best solution. What do you think?
OK, I see now that it uses pointers to be able to modify the actual KFileItems in KDirListerCache (if it would just keep KFileItems and modify these, I guess that they would just detach from the ones inside the cache, leaving those unchanged). It looks like a solution for this problem is more complicated than I thought, so maybe I'll just revert the commit to make Jenkins happy again. However, I still think that making KFileItem a Q_MOVABLE_TYPE might be a desirable long-term goal because it saves quite a bit of memory (32 bytes per KFileItem on my system). Regards, Frank _______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
