[Libreoffice] [REVIEW 3-4] fdo#39510 crash on closing document with footnotes

[Mathias Bauer]

Am 24.09.2011 09:57, schrieb Michael Meeks:

> 
> On Sat, 2011-09-24 at 00:36 +0200, Mathias Bauer wrote:
>> Really, the layout is a complete mess, at least from an
>> architectural POV. There is no clear relationship and ownership between
>> objects. That might work better in a world with garbage collection, but
>> in the ugly world of pointers this is the road to hell.
> 
> 	Hey :-) first - many thanks for your review, and great to see you
> here !
> 
> 	I just did a size-able cleanup of color pieces to use
> reference-counting everywhere - fixing a number of rather hideous
> tangled lifecycle issues in the process, and I was wondering, would it
> make sense to add a ref-count + 'dispose' pattern to these Layout
> objects in a big one-shot re-factor ?

I expect that just adding refcounting for all layout objects here would
lead to cyclic references and so probably would just replace one problem
with another. So surely some of the involved references would need to be
weak references.

If I'm not wronf, that's probably a different way to express the
"refcount + dispose" pattern you have mentioned. Using weak refs
requires checking them before usage, OTOH you don't rely on a proper
implementation of the usage and implementation of dispose() calls and
you don't need to pave your code with Dispose() calls. Probably a matter
of taste.

The differentiation between hard and soft refererences would require a
solid understanding of the relationships of all objects. With this
understanding you could most probably fix the existing code without
refcounting also: a weak reference always could be replaced by a
callback function that tells possible consumers of an object that this
object is going to die (and so manually sets the pointer to Null), a
pattern that can be found in OOo's code in other places (e.g.
SfxBroadcaster or SwNotify objects). But of course using refcounting
here would make the code clearer.

The biggest part of the work is getting the right picture: which objects
control the life time of others and which just need to know if some
other objects are present or not.

In the footnotes example obviously some objects have pointer of other
objectes that are already destroyed. We had to decide if they either
need to keep these objects alive until they are done with them or if
they just had to keep a weak reference to them to avoid the crash we are
seeing now.

Just my 2 cents.

Regards,
Mathias