Boost logo

Threads-Devel :

From: Roland Schwarz (roland.schwarz_at_[hidden])
Date: 2006-10-06 03:41:57


The Problem:
============

Initialisation of local static objects is
potentially not thread-safe.

E.g.:

class bar
{
public:
     bar()
     {
         i = 42;
     };
private:
     int i;
};

void foo()
{
     static bar abar;
}

The standard says (6.7/4):
" ... Otherwise such an object is initialized
the first time control passes through its
declaration; ... If control re-enters the
declaration (recursively) while the object is
beeing initialzed, the behaviour is undefined."

 From this it follows that the constructor call
poses a race condition, when a second thread is
making a call to foo, while the first still is
in the constructor of bar.

Side Note:
==========

My research was triggered by a bug in the spirit
library:
"boost/spirit/core/non_terminal/impl/grammar.ipp"
Ref:
http://lists.boost.org/Archives/boost/2006/04/102794.php
Ref:
http://sourceforge.net/tracker/index.php?func=detail&aid=1509978&group_id=7586&atid=107586

In a templated function

     template<typename DerivedT, typename ContextT, typename ScannerT>
     inline typename DerivedT::template definition<ScannerT> &
     get_definition(grammar<DerivedT, ContextT> const* self)
     {
         ...
         typedef grammar<DerivedT, ContextT> self_t;
         typedef impl::grammar_helper<self_t, DerivedT, ScannerT>
             helper_t;
         typedef typename helper_t::helper_weak_ptr_t ptr_t;

         static boost::thread_specific_ptr<ptr_t> tld_helper;
         if (!tld_helper.get())
             tld_helper.reset(new ptr_t);
         ptr_t &helper = *tld_helper;
         ...
     }

a local static thread_specific pointer is beeing
used for thread safety. To make this really thread
safe the pointer normally should be declared at
global (namespace) scope. This is not possible in
this case, since the pointer type is parameterized
on a template parameter.

Solution(s):
============

1)--
     The simplest, straight forward solution, would
be, to guard the static declaration by a global mutex.
I.e.:

mutex::mx;

template<C> void foo(C arg)
{
     mutex::scoped_lock lk(mx);
     static bar<C> abar;
     lk.unlock();
     ...
}

However this has some drawbacks:
*) All templated (unrelated) functions share a
     single mutex.
*) All function calls need to pass through a
     single point of synchronization, altough
     not necessary, once the object is initialized.

2)--
     Make use of a statically initializeable mutex.

template<C> void foo(C arg)
{
     static_mutex mx = STATIC_MUTEX_INITIALIZER;
     mutex::scoped_lock lk(mx);
     static bar<C> abar;
     lk.unlock();
     ...
}

While this removes the first of the above drawbacks,
the second still applies. (A static_mutex can be found
e.g. in boost/regex/pending/static_mutex.hpp)

3)--
     Use a static pointer to the object and initialize
on first use, by making use of boost::call_once.
Based on this idea I wrote a little class wrapper that
can be used as following:

template<C> void foo(C arg)
{
     static once_class<bar<C> > static_abar = ONCE_CLASS_INIT;
     bar<C>& abar(static_abar);

     ...
     abar.blah();
     ...

}

Notes:
Uses of this wrapper, not only cover the above case, but
can be used to make any object statically initializeable.
The only precondition is that it has a default ctor.

In particular it makes it unnecessary to create static init
versions of the various mutex types.

I.e.:

void foo()
{
     static once_class<boost::mutex> mx = ONCE_CLASS_INIT;
     boost::mutex::scoped_lock lk(mx);

     ....

}

Or

void foo()
{
     static once_class<boost::recursive_mutex> mx = ONCE_CLASS_INIT;
     boost::mutex::scoped_lock lk(mx);

     ....

}

Conclusion:
===========
Solution 3) does not suffer from the single point
of synchronization drawback and can make any default
constructible object statically initialzeable in a
thread-safe manner.
This of course can also be used to declare a (global)
mutex that needs to be available from within global
ctor's.

I have attached a commented implementation of this
class wrapper together with a sample usage.

I would be glad to get feedback on this design, before
adding it to the boost::thread library.

Regards,
Roland


// Copyright (C) 2006 Roland Schwarz
//
// Distributed under the Boost Software License, Version 1.0. (See accompanying
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

#ifndef ONCE_CLASS_HPP__
#define ONCE_CLASS_HPP__

#include <boost/thread/once.hpp>
#include <boost/thread/mutex.hpp>
#include <boost/thread/tss.hpp>
#include <vector>

#define ONCE_CLASS_INIT {BOOST_ONCE_INIT,0}
// once_class _is_ an aggregate, so it can be statically
// initialized. Since it also has no destructors, the
// compiler need not schedule a call to it (which might
// create a race condition).
template<class A>
struct once_class
{
    // The conversion operator allows direct access to the
    // wrapped class, and constructs it on demand in a
    // thread safe manner.
    // N.B.: This does not make the class thread safe,
    // only construction is. You must supply your own
    // synchronization to make it thread safe.
    operator A&() {
        boost::call_once(&init_class, class_flag);
        // Using a thread global variable to pass arguments,
        // since call_once does not allow passing arguments
        // directly.
        //once_class_arg->reset(reinterpret_cast<void**>(&p));
        arg->reset(&instance);
        boost::call_once(&init_instance,instance_flag);
        return *instance;
    };
    boost::once_flag instance_flag;
    A* instance;
private:
    static void init_instance()
    {
        //A** ppm = reinterpret_cast<A**>(once_class_arg->get());
        A** ppa = arg->get();
        *ppa = new A;
        boost::mutex::scoped_lock lk(*guard);
        instances->push_back(*ppa);
    };
    static void init_class()
    {
        arg = new boost::thread_specific_ptr<A*>(no_cleanup);
        guard = new boost::mutex;
        instances = new std::vector<A*>;
        // schedule removal
        // NOTE: a once_class must not be used from any functions
        // that will run from an atexit handler!
        atexit(&cleanup);
    };
    static void cleanup()
    {
        std::vector<A*>::size_type i;
        for (i=0; i<instances->size(); ++i)
        {
            delete (*instances)[i];
        }
        delete instances;
        delete guard;
        delete arg;
    };
    static void no_cleanup(A**) {};
    static boost::once_flag class_flag;
    static boost::thread_specific_ptr<A*>* arg;
    static boost::mutex* guard;
    static std::vector<A*>* instances;
};

// Making the following pointers, allows usage of
// once_class from within "static" ctor calls.
// (But I would not recommend such usage.)
template<class A> boost::once_flag once_class<A>::class_flag = BOOST_ONCE_INIT;
template<class A> boost::thread_specific_ptr<A*>* once_class<A>::arg = 0;
template<class A> boost::mutex* once_class<A>::guard = 0;
template<class A> std::vector<A*>* once_class<A>::instances = 0;

#endif // ONCE_CLASS_HPP__

// Copyright (C) 2006 Roland Schwarz
//
// Distributed under the Boost Software License, Version 1.0. (See accompanying
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

#include "once_class.hpp"

#include <boost/thread/mutex.hpp>
#include <boost/thread/thread.hpp>
#include <iostream>
using namespace std;

class test {
public:
    test() {
        boost::mutex::scoped_lock lk(m);
        cout << "constructor" << endl;
    };

    ~test() {
        boost::mutex::scoped_lock lk(m);
        cout << "destructor" << endl;
    };
    void operator()(){
        boost::mutex::scoped_lock lk(m);
        cout << "called" << endl;
    };
private:
    boost::mutex m;
};

void foo()
{
    static once_class<test> static_bar = ONCE_CLASS_INIT;
    test& bar(static_bar);

    bar();
}

int main(int argc, char *argv[]) {
    boost::thread th1(foo);
    boost::thread th2(foo);
    th1.join();
    th2.join();
}


Threads-Devel list run by bdawes at acm.org, david.abrahams at rcn.com, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk