Re: [Boost-users] [multi-index] Container not safe against reentrant clear() / erase() ?

Subject: Re: [Boost-users] [multi-index] Container not safe against reentrant clear() / erase() ?
From: Sebastian Theophil (stheophil_at_[hidden])
Date: 2009-06-04 05:17:30

> That's what I find hard to assume: how is it permissible to
> destroy a CObject at any time when its refcount is not
> zero? If you have some object whose lifetime is managed by
> boost::shared_ptr, I think it is a bad idea to mess with
> the object lifetime directly.
> 
> Please note this is not a rhetoric question: how do you know
> when you explicitly destroy a CObject that you leave no
> dangling boost::shared_ptrs to it?

We do not use the boost::shared_ptr class and its relatives but our own reference counting implementation. There are two kinds of references, owned_ref<T> represents an "owning" relationship between a parent and a child. If such a reference is destroyed, it deletes the object it points to. ref<T> is a simple reference that increases the refcount of T. If the last ref of either type is destroyed, the object is always destroyed. Some objects are kept alive only by refs, other objects are owned and their lifetime is bound to the lifetime of their parent. We reference count all objects but this doesn't necessarily imply that every reference should keep an object alive. It only implies that every such reference must be removed when the object is destroyed. After all dtors have run, the refcount must be zero and this is asserted. It doesn't have to be zero when the dtor is called. The object itself manages its lifetime, not the references to it. 

Imagine you have a hierarchical application data model e.g.

CPresentation
|_ CSlide
   |_ CChart
     |_ CDataLabel

This application data model doesn't change very often. Changing the data model causes changes to the file format, the forward and backward conversion, possibly the user interface etc. Every object's lifetime and the collections in which it is held are very clearly defined and are known to the object itself. The question what if somebody uses CDataLabel in a totally unforeseen way doesn't really apply. 

Each parent owns his children. 
- The parent constructor constructs the child objects, the child objects may register in some collection higher up the chain. All elements with a user interface may register with some top level object that implements the mouse handling for example.

- The parent destructor causes the destructions of its child objects. The child dtor deregisters the child object at all places where it registers in the ctor.

In my opinion this is a simple structure. Object construction and destruction are inverse operations of each other, written side by side. Object registration and deregistration only happen at one place, so nobody can forget to do this in the right order, or at the right place etc.

Now there is one additional use case: An object like CDataLabel may be destroyed at any time because the user hit delete or pressed a button, or called a macro function. This is forwarded to the corresponding user interface object which simply deletes its owner, ie the data label itself. The user interface class could ask the parent to delete a label of course, but this becomes impractical if there are different labels with different parents but the same user interaction. Again, if each (specialized) CDataLabel dtor can remove all references to itself, no caller can forget to do it, the code remains pretty simple, and the object tree is only managed at one place.

The CDataLabel dtor removes the object from the class hierarchy described above, i.e., the dtor also deregisters the object from its parent. When the parent itself is destroyed, this deregistration is a noop because the data label reference is already destroyed. 

> The latter is the way the lib currently works. Are you experiencing
> otherwise?

If the CChart has a multi index container of owning references to data labels, the problem I've described could result. If the multi_index_container is cleared and each reference is released, the CDataLabel is destroyed. During clear() you would have a stack trace like this:

multi_index_container::clear()
- delete_node(root())
- destroy(node*)
- ~owned_ref< CDataLabel >
- ~CDataLabel
-> and at this point the half-destructed reference is still part of the multi_index_container. Because it is half-destructed, the ordering criterion of the index may be violated and the CDataLabel cannot make a lookup on the container, neither to make sure it is removed itself, nor to remove e.g. dependent labels. Lookups may crash in fact because the index assumes an ordering relation between the left/right/parent nodes and runs off into uninitialized memory.

Regards,
Sebastian

--
Sebastian Theophil · stheophil_at_[hidden]
Software Engineer
 
think-cell Software GmbH · Invalidenstr. 34 · 10115 Berlin, Germany 
http://www.think-cell.com · phone [phone] · toll-free (US) [phone]
Directors: Dr. Markus Hannebauer, Dr. Arno Schoedl · Amtsgericht Berlin-Charlottenburg, HRB 85229