From kde-devel Thu Dec 12 16:07:17 2013 From: Kevin Funk Date: Thu, 12 Dec 2013 16:07:17 +0000 To: kde-devel Subject: Re: Problem with KCompTreeNode on Windows (or: destruction order of static objects?) Message-Id: <1769915.WXVhrr4I70 () kerberos> X-MARC-Message: https://marc.info/?l=kde-devel&m=138686447522921 Am Donnerstag, 12. Dezember 2013, 11:27:07 schrieb Frank Reininghaus: > Hi, > > 2013/12/11 Kevin Funk: > [...] > > >> > > Just using K_GLOBAL_STATIC for the KZoneAllocator instance doesn't > >> > > help > >> > > either. I get the same destruction order + crash with that on > >> > > Windows. > >> > > > >> > > Dynamic allocation inside KCompTreeNode doesn't work either, because > >> > > KZoneAllocator needs to be there *before* the first KCompTreeNode is > >> > > created and needs to there *until* the last one has been destroyed. > >> > > > >> > > Any ideas? > >> > > >> > Well, here is one idea (untested): use a > >> > QSharedPointer to ensure that the KZoneAllocator is > >> > not destroyed before the last KCompletion instance using it. > >> > > >> > Replace > >> > > >> > static KZoneAllocator alloc; > >> > > >> > in KCompTreeNode by > >> > > >> > static QSharedPointer alloc; > >> > > >> > and, in kcompletion.cpp, > >> > > >> > KZoneAllocator KCompTreeNode::alloc(8192); > >> > > >> > by > >> > > >> > QSharedPointer KCompTreeNode::alloc; > >> > > >> > In KCompTreeNode's operator new/delete, replace "alloc." by "alloc->". > >> > > >> > Furthermore, add a QSharedPointer member to > >> > KCompletionPrivate. In KCompletionPrivate's constructor, first check > >> > if KCompTreeNode::alloc isNull(), initialize it if that is not the > >> > case, and then copy "alloc" to the new member. > >> > > >> > I think that this might work, and it might also improve application > >> > start-up time because the KZoneAllocator would only be initialized on > >> > first use of a KCompletion object. > > > > Hey Frank, > > > > thanks a lot for sharing your thoughts! > > > > I just tried your suggestion, and it indeed fixes the issue on Windows > > (and > > doesn't cause havoc on Linux either). See attached patch (v1). I think it > > pretty much resembles your thoughts. > > great, I'm glad to hear that! I'd recommend to remove the unrelated > changes in KZoneAllocator though - they just make the patches harder > to understand. Unfortunately it's not really unrelated -- When using qDebug() in ~KZoneAllocator, that sometimes seems to crash in deep inside Qt (QTextCodec/QIconvCodec) during __cxa_finalize. Apparently it is too late to call qDebug() at this point. (Reduced) Backtrace: #0 malloc_consolidate (av=av@entry=0x7ffff6061740 ) at malloc.c:4088 #1 0x00007ffff5d210e1 in _int_malloc (av=0x7ffff6061740 , bytes=32640) at malloc.c:3379 #2 0x00007ffff5d234d0 in __GI___libc_malloc (bytes=32640) at malloc.c:2859 #3 0x00007ffff5cc2b19 in __gconv_open (toset=0x44db30 "\002", toset@entry=0x7fffffffd660 "//", fromset=0x7fff00000000
, fromset@entry=0x7fffffffd640 "UTF-16//", han dle=handle@entry=0x7fffffffd680, flags=flags@entry=0) at gconv_open.c:283 (and further:) #4 0x00007ffff5cc24a1 in iconv_open #5 0x00007ffff74703b2 in QIconvCodec::createIconv_t #6 0x00007ffff746fd13 in QIconvCodec::convertFromUnicode #7 0x00007ffff7469aa7 in QTextCodec::fromUnicode #8 0x00007ffff7346b24 in QString::toLocal8Bit #9 0x00007ffff76a10ac in QDebug::~QDebug #10 0x00007ffff77da9f6 in KZoneAllocator::~KZoneAllocator #11 0x00007ffff7b82e48 in QtSharedPointer::ExternalRefCount::deref > > However, I'm not quite sure this approach is going to be accepted > > upstream. > > One issue, for example, is that KCompTreeNode now has a publicly > > accessible > > static KZoneAllocator* member and explicitly setting the allocator from > > within KCompletion seems odd. That completely makes KCompTreeNode(s) > > depend on KCompletion. > > > > I'm proposing another approach: > > - Still using QSharedPointer > > - But: Initialize this shared-pointer directly (as with the static KZA > > member) - And: Keep a strong-ref in KCompletion on that shared pointer > > See attached patch (v2) > > > > Any objections? Otherwise I'll post that patch to reviewboard ASAP. > > well, I guess it might be considered a matter of taste. The reason why > I find v1 slightly better is that the initialization of the custom > allocator is deferred until it is really needed. Right now, the > KZoneAllocator constructor will be called on startup for every > application that links to kdeui. > > BTW, I found the discussion about the introduction of KZoneAllocator > in KCompletion: > > http://lists.kde.org/?t=101548827800001&r=1&w=2 > > The author claims that > > "A changed kcompletiontest which just add a big number of strings in > different ways, and clears the completion object in between showed more > than a double speedup (from 4950 to 2100 msec)" > > However, it seems that this "changed" test is not provided, and > moreover, the patch changes a lot more than just the allocator, so > there is no way to know which part of the saving is due to which > change. IMHO, two good reasons why this patch should not have been > committed without further discussions, but I guess that my objection > is more than 11 years too late ;-) > > (BTW, if I wrote a custom allocator for a class which always allocates > objects of the same size, I would try to make use of that in order to > get rid of all the overhead that is needed to keep track of the size > of the allocated blocks, but that's not the topic of this thread.) > > Cheers, > Frank Interesting. Thanks for your help in this regard! I'll post the patch to reviewboard in the near future. Cheers -- Kevin Funk >> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<