Re: [boost] [flyweight][#3658] Dependency on static initialization/destruction order

Subject: Re: [boost] [flyweight][#3658] Dependency on static initialization/destruction order
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2009-11-28 12:14:55

Joaquin M Lopez Munoz wrote:
> 
> Singletons (which is what Boost.Flyweight holder finally boil
> down to, see http://tinyurl.com/y996eqr ) are notoriously hard to
> get right, as explained for instance in Alexandrescu's "Modern
> C++ Design", and in the end one has to settle for some compromise.
> Boost.Flyweight uses holders in such a way that it guarantees
> initialization before main() begins, which for the most part avoids
> problems with lazy initialization in multithreaded environments.

Yes, I'm familiar with problems regarding singleton initialization and 
destruction. In fact, I'm quite satisfied with the current holder 
initialization (although, it could have used call_once to eliminate 
thread safety issues, but that's another topic).

> Would you mind sketching what replacement for static_holder_class
> you have in mind?

I can imagine at least two ways of achieving the requested fix. The 
first one is to let the held factory to outlive the holder. Something 
along these lines:

     template< typename C >
     struct static_holder_class
     {
         static C& get()
         {
             static shared_ptr< C > instance;
             if (!instance)
                 instance = make_shared< C >();
             return *instance;
         }
     };

Or better yet:

     template< typename C >
     struct static_holder_class
     {
         static shared_ptr< C > get()
         {
             static once_flag flag = BOOST_ONCE_INIT;
             call_once(flag, &static_holder_class::get_impl);
             return get_impl();
         }

     private:
         static shared_ptr< C > get_impl()
         {
             static shared_ptr< C > instance;
             if (!instance)
                 instance = make_shared< C >();
             return instance;
         }
     };

Now, the values created by factory should also hold a reference to the 
factory. In the first code snippet this would require C to derive from 
enable_shared_from_this. This will increase the value footprint a 
little, I know (one pointer, if intrusive_ptr is used).

The second solution is to let the factory die, but to leave the values 
live as long as there are flyweights referencing them. This does not 
require changing the holder as its implementation is orthogonal to the 
factory nature.

I can't say for sure what exact part of the library should be modified 
to achieve this, but at least the value factory and refcounted tracking 
policy would have to be changed. Boost.Intrusive would help to craft the 
container to store values in. Here is a snippet to get the idea:

     typedef intrusive::set_base_hook<
         intrusive::link_mode< intrusive::auto_unlink >,
         intrusive::optimize_size< true >
     > hook_t;

     template< typename ValueT >
     struct value_holder :
         public hook_t
     {
         atomic_count m_RefCount;
         ValueT m_Value;

         friend void intrusive_ptr_add_ref(value_holder*);
         friend void intrusive_ptr_release(value_holder*);
     };

     template< typename ValueT >
     struct factory
     {
         typedef intrusive::set<
             value_holder,
             intrusive::base_hook< hook_t >
         > container_t;

         mutex m_Mutex;
         container_t m_Container;

         ~factory();
         intrusive_ptr< value_holder< ValueT > > insert(ValueT const&);
         void erase(intrusive_ptr< value_holder< ValueT > > const&);
     };

The intrusive_ptr would have to be stored in the flyweights.

The key points here would be release and ~factory. The destructor should 
call m_Container.clear(), which unlinks all values but leaves them 
valid. The release function, aside from decrementing the reference 
counter and destroying the value, has to check if the value is linked or 
not. If it is, the object is still in the factory container, and it 
should call factory::erase. If not, the value is not bound with the 
container and can be simply deleted.

> PS: There's an additional approach that can solve the issue
> at hand, namely implementing a holder that never destroys
> the held object, but this is something I'd like to discuss
> with you after your original refcount proposal is dealt with.

Yes, that's a possible solution, but it would introduce a memory leak, 
which I'd like to avoid.