[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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic