incubator-ooo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andre Fischer ...@a-w-f.de>
Subject Re: [CODE] issue 118576: Crash on close
Date Mon, 07 Nov 2011 09:53:02 GMT


On 07.11.2011 10:24, eric b wrote:
>
> Le 7 nov. 11 à 09:51, Andre Fischer a écrit :
>
>> Hi,
>>
>
>
> Hi Andre,
>
>
>> I have news on this, but not good news.
>>
>
>
> :-/
>
>
>> The crash is not directly caused but triggered by the fix for issue
>> 112786. That issue fixes the config manager singleton to actually
>> getting destroyed when the office closes. Not destroying it is not
>> nice but, to my knowledge, causes not observable problems.
>
>
> That's probably why Caolan workaround uses boost shared_pointers. I
> adapted his patch to OOo, and there is no observable problem on Windows
> so far.
>
>>
>> Now that the config manager is destroyed, it checks whether there are
>> still any registered config items. There should be none at this point
>> but in reality there is one when no application is started and about
>> six when for example the writer is started and closed immediately.
>
>
> I didn't know the Writer case. Thanks for the info.
>
>
>>
>> Not wanting to leave anything behind the config manager destroys the
>> lingering config items. Because the office is already half down and
>> the infrastructure for destroying the config items is not in place
>> anymore, that causes the crash.
>
> Indeed.
>
>
>>
>> So, every remaining config item causes its own separate crash. Each
>> has to be fixed on its own. Some even cause secondary crashes when a
>> config items in its destructor tries to destroy the resources it still
>> holds.
>> The root cause here is that the config items are registered but are
>> not unregistered.
>
>
> Yes, so why not use shared pointers ? the refcount looks more clean, is
> automagicaly managed, and works apparently well.
>
> Or maybe I misunderstood something ?
>
>
>
>> Fixing this issue means finding the owner of the offending config item
>> and make it properly unregister and destroy the item.
>>
>> Fixing this is a time consuming process.
>
>
> Indeed, and will force to manage more precisely every item.
>
>
>> I have one item fixed (see the patch for issue 118576) and a second
>> one in the making. That means that four are still open. As there are
>> more important things to do right now, like getting the conflicting IP
>> stuff removed, I suggest to remove the patch of issue 112786 and fix
>> this properly at a later point in time.
>>
>
>
> Waiting, what about adopt a compromise, using the patch I wrote (or a
> better one) instead ? See :
> http://ftp.educoo.org/home/ericb/patches/apache_ooo/configmgr_windows/configmgr_fixed.diff

 From what I can see at a first glance, this fixes only the symptoms but 
not the root cause.  That, however, is not a bad thing, since the root 
cause exists since before issue 112786 was fixed.

The config items still exist when the config manager is destroyed.  They 
should have been removed before that.  Their life time control does not 
work.  Boost shared_ptrs might help here.  The owners of the config 
items (in the two cases I investigated so far) use their own, hand-made 
reference counting, sometimes two layers deep.  Replacing this with 
shared_ptrs or scoped_ptrs might fix this.  But once you start cleaning 
up this old code you won't be able to stop.


>
>
> Another question coming to my mind is now: can we integrate the patch,
> or must we write another solution ? (I tried to discuss with Caolan on
> IRC, but no answer yet)
>
>
> Thanks in advance for any suggestion :-)

If your patch fixes the crash(es) then that is as good as removing the 
offending patch of issue 112786.

When we have a little time at our hands then we still should fix the 
life time control of the config items.

Regards,
Andre

>
>
> Regards,
> Eric
>
>
> Note : I manualy added the changes, since the patch Caolan provided uses
> deep changes in cppuhelper, not directly compatible with our code imho.
> The orignal link of the fix is :
> https://bugs.freedesktop.org/show_bug.cgi?id=31494
>
>

Mime
View raw message