Jenkins build became unstable: kdelibs_frameworks_qt5 #982

[Frank Reininghaus]

Hi,

2013/8/6 Frank Reininghaus:
> Hi,
>
> 2013/8/6 KDE CI System <null at 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