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

Hi, I've created the ticket #3658 [1] on the flyweights library. The ticket is about Boost.Flyweight depending on the static initialization/destruction order, which can lead to application crashes in some cases. The library does provide certain tools to work around the problem [2], but these tools are only usable in the most obvious cases and do not solve the problem in its root. What I have proposed in the ticket is to implement reference counting in the flyweights, so that the flyweight value is only destroyed when no flyweight handles refer to it. AFAIK, Boost.Flyweight already has reference counting, so the work is only to shift value owning from the holder to the flyweights. Comments are welcome. [1] https://svn.boost.org/trac/boost/ticket/3658 [2] http://www.boost.org/libs/flyweight/doc/tutorial/technical.html#static_init

Andrey Semashev <andrey.semashev <at> gmail.com> writes:
Hi,
I've created the ticket #3658 [1] on the flyweights library. The ticket is about Boost.Flyweight depending on the static initialization/destruction order, which can lead to application crashes in some cases.
The library does provide certain tools to work around the problem [2], but these tools are only usable in the most obvious cases and do not solve the problem in its root.
Hi Andrey, 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.
What I have proposed in the ticket is to implement reference counting in the flyweights, so that the flyweight value is only destroyed when no flyweight handles refer to it. AFAIK, Boost.Flyweight already has reference counting, so the work is only to shift value owning from the holder to the flyweights.
I'm not sure what's the exact proposal you have. Take into account that refcounts applies to flyweight values, while holders (as defined in Boost.Flyweight) deal with initializing/destroying the flyweight value *factory*, that is, the object that holds the values. This is basically the implementation of static_holder, the default holder for Boost.Flyweight: template<typename C> struct static_holder_class { static C& get() { static C c; return c; } }; Would you mind sketching what replacement for static_holder_class you have in mind? Thank you, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo 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.

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.
participants (2)
-
Andrey Semashev
-
Joaquin M Lopez Munoz