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