Modify

Ticket #2359 (closed Bugs: fixed)

Opened 6 years ago

Last modified 22 months ago

Static initialization problems with fast_pool_allocator

Reported by: cnewbold Owned by: cnewbold
Milestone: Boost 1.37.0 Component: pool
Version: Boost 1.36.0 Severity: Problem
Keywords: Cc: joaquin@…

Description

Detailed analysis from Joaquín M López Muñoz:

Hello, I did look hard into pool/detail/singleton.hpp in the past because it deals with a problem that I also had to face in the implementation of Boost.Flyweight. These are my conclusions wrt to pool/detail/singleton.hpp and with respect to the particular problem you're now studying:

  • pool/detail/singleton.hpp claims that singleton_default<T>::instance() is automatically called before main() if no user code does it explicitly. Strictly speaking, the implementation does not guarantee this, but only that singleton_default<T>::instance() will be called during the so-called dynamic initialization phase of the program startup sequence. For all practical purposes, however, this is equivalent to the orignal claim, so the problem does not lie here.
  • Where the problem lies can be found by looking at the usage of singleton_pool by fast_pool_allocator: notice that singleton_pool is used *only* when fast_pool_allocator::allocate or ::deallocate are invoked. So, it is perfectly possible to construct a fast_pool_allocator without that forcing the compiler to construct the underying singleton_pool *yet*. And this is what's happening indeed, the sequence of objects construction we're having is this:
  1. owned_base::pool_ begins construction

1.1 call of new pool_lii() inside pool::pool

1.1.1 a fast_pool_allocator is cted inside pool_lii ctor.

1.2 pool_ii ends construction

  1. pool_ ends construction
  1. the singleton instance associated to singleton_pool<...>is

cted before main() because fast_pool_allocator uses it later

(singleton guarantee).

This sequence is consistent with the guarantees provided by singleton_default<T>. The problem lies in the design of

fast_pool_allocator: as it stands, the construction of a fast_pool_allocator does *not* force the prior construction of the internal singleton instance, and this is a mistake.

If my analysis is correct, to fix the problem one need only ensure that his construction order is preserved for instance by explicitly using singleton_pool<...> inside fast_pool_allocator ctors like this:

    fast_pool_allocator()

    {

      singleton_pool<fast_pool_allocator_tag, sizeof(T),

        UserAllocator, Mutex, NextSize>::is_from(0);

    }



    template <typename U>

    fast_pool_allocator(

        const fast_pool_allocator<U, UserAllocator, Mutex, NextSize> &)

    {

      singleton_pool<fast_pool_allocator_tag, sizeof(T),

        UserAllocator, Mutex, NextSize>::is_from(0);

    }

Attachments

Change History

comment:1 Changed 6 years ago by cnewbold

I'm not sure I buy this explanation entirely.

owned_base::pool_ is a global object. As described above, so is object_creator.

The test case consists of a single translation unit, and inside that translation unit, the declaration of object_creator precedes that of owned_base::pool_ (because object_creator is declared by a Boost.Pool header which is included before the application-level code which declares owned_base).

Following the fairly clear rules about initialization order for namespace-scoped global objects, I would have expected the compiler to construct object_creator first (and therefore the pool) before constructing owned_base::pool_.

I'm not at all convinced that singleton_default or fast_pool_allocator is actually correct, but I do think this situation is a bit more complex...

More from Joaquín

These precedence rules do not apply to class template static data: quoting the standard (latest draft at  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2691.pdf), section 3.6.2:

"Dynamic initialization of a non-local object with static storage duration is either ordered or unordered. Definitions of explicitly specialized class template static data members have ordered initialization. Other class template static data members (i.e., implicitly or explicitly instantiated specializations) have unordered initialization. Other objects defined in namespace scope have ordered initialization. Objects with ordered initialization defined within a single translation unit shall be initialized in theorder of their definitions in the translation unit."

That is, only *ordered* file-scope objects are subject to the precedence rule you're referring to. Class template static data are not ordered by definition (except if belonging to a full specialization). So, the sequence I proposed above is consistent AFAICS.

comment:2 Changed 6 years ago by cnewbold

Ah! This is just the sort of explanation I was looking for, but could not find--I guess my copy of the C++ standard has gone stale.

Based on this, it certainly would appear that GCC's behavior is conformant and that the combination of fast_pool_allocator and singleton_default presumes behavior which is unspecified.

My only concern is whether referencing singleton_default from the constructor for fast_pool_singleton will guarantee proper initialization ordering.

3.6.2 doesn't really shed any light on how ordered and unordered initialization may be coupled. There isn't, for example, any expressed guarantee that non-locals will be initialized prior to first reference.

More from Joaquín:

There is no such guarantee indeed, but the fix does NOT rely on initialization rules for non-local objects, but on the initialization rules for *local* objects with static storage duration (6.7/4): the singleton is a local static object inside the function singleton_default<T>::instance(), which is explicitly invoked inside fast_pool_allocator ctors via singleton_pool<...>is_from() (if you apply the fix, that is).

The non-local object involved in singleton_default<T>, namely singleton_default<T>::create_object is there to guarantee that singleton_default<T>::instance() is invoked, and thus the singleton created in dynamic initialization time *if no one has done it before*.

comment:3 Changed 6 years ago by cnewbold

And finally...

Quite right. I now agree your suggested fix is completely sound.

And, in fact, making your suggested fix re-orders the construction of the pool and allocator objects as expected. I tried the experiment with GCC 4.1.2.

The next step is for me to audit the remainder of the pool library and see if there are other clients of singleton_pool which are subject to the same potential failure mode.

comment:4 Changed 6 years ago by cnewbold

  • Status changed from new to assigned

comment:5 Changed 6 years ago by cnewbold

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed on trunk by revision 49025.

comment:6 Changed 3 years ago by anonymous

singletons are evil. This is my mind...

comment:7 Changed 22 months ago by sehmann@…

Can you please address this performance issue? We are seeing a 35% slowdown on our code when using fast_pool_allocator as a result of this bug fix.  http://lists.boost.org/Archives/boost/2009/07/154257.php

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.