----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114715/#review47255 -----------------------------------------------------------
As discussed on the mailing list, I think that this is a valid approach to ensure that the custom allocator is not deleted before the last KCompletion instance using it. The change might not only be beneficial on Windows - maybe the code is a time bomb waiting to explode also on other platforms. I think it would be slightly nicer to initialize the allocator when it's first used, rather than when the library is loaded, but this is unrelated to the purpose of this patch. Maybe something to consider for Frameworks, to which this patch must be ported manually anyway? (Another question for Frameworks would obviously be how much the performance is actually improved by the custom allocator, and thus, if it's worth keeping it. As I said in http://lists.kde.org/?l=kde-devel&m=138684405315855&w=1, there were some claims in 2002 that it greatly speeds up KCompletion, but I'm not sure if these claims can (still) be trusted). In any case, I don't have good knowledge of KCompletion, so I don't feel qualified to say "Ship It!" (at least not before waiting another week or two for possible comments from others). - Frank Reininghaus On Jan. 3, 2014, 5:52 p.m., Kevin Funk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114715/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2014, 5:52 p.m.) > > > Review request for kdelibs and Frank Reininghaus. > > > Bugs: 327599 > http://bugs.kde.org/show_bug.cgi?id=327599 > > > Repository: kdelibs > > > Description > ------- > > Attempt to fix KZoneAllocator issue > > kcompletion.p_h: Make the static KZoneAllocator member of KCompTreeNode > a shared pointer so external users can hold a reference to it. > > kcompletion.cpp: Hold a reference to KCompTreeNode's KZoneAllocator > instance so we avoid deleting the KZoneAllocator too early. See attached > bug report for possible causes. (Hint: It crashes on Windows because > ~KZoneAllocator is called to early.) > > kallocator.cpp: Use printf instead of qDebug(), because this code path > code might be called very late during destruction and qDebug() will > crash deep inside Qt. > > Also see discussion: > http://lists.kde.org/?l=kde-devel&m=138583383708455&w=1 > > BUG: 327599 > CCBUG: 243375 > > > Diffs > ----- > > kdecore/util/kallocator.cpp 8b21120c62c513ea41686fe8185ec2808fe5d83a > kdeui/util/kcompletion.cpp 340aa92b900d670e2ad73f70a63d5221d0feed1d > kdeui/util/kcompletion_p.h 1cf31db3f16fe3421415cd54265eee20bb998710 > > Diff: https://git.reviewboard.kde.org/r/114715/diff/ > > > Testing > ------- > > > Thanks, > > Kevin Funk > >
