[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: Problem with KCompTreeNode on Windows (or: destruction order of static objects?)
From:       Kevin Funk <krf () gmx ! de>
Date:       2013-12-12 16:07:17
Message-ID: 1769915.WXVhrr4I70 () kerberos
[Download RAW message or body]

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<KZoneAllocator> to ensure that the KZoneAllocator is
> >> > not destroyed before the last KCompletion instance using it.
> >> > 
> >> > Replace
> >> > 
> >> > static KZoneAllocator alloc;
> >> > 
> >> > in KCompTreeNode by
> >> > 
> >> > static QSharedPointer<KZoneAllocator> alloc;
> >> > 
> >> > and, in kcompletion.cpp,
> >> > 
> >> > KZoneAllocator KCompTreeNode::alloc(8192);
> >> > 
> >> > by
> >> > 
> >> > QSharedPointer<KZoneAllocator> KCompTreeNode::alloc;
> >> > 
> >> > In KCompTreeNode's operator new/delete, replace "alloc." by "alloc->".
> >> > 
> >> > Furthermore, add a QSharedPointer<KZoneAllocator> 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 <main_arena>) at 
malloc.c:4088
#1  0x00007ffff5d210e1 in _int_malloc (av=0x7ffff6061740 <main_arena>, 
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 <Address 
0x7fff00000000 out of bounds>, 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<KZoneAllocator>::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<KZoneAllocator>
> > - 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 <<
[prev in list] [next in list] [prev in thread] [next in thread]