Re: [boost] [thread] Can Boost.Thread use Boost.Atomic without falling on a compatibility issue?

Subject: Re: [boost] [thread] Can Boost.Thread use Boost.Atomic without falling on a compatibility issue?
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-01-13 13:32:04

Le 13/01/13 18:51, Andrey Semashev a écrit :
> On Sun, Jan 13, 2013 at 9:12 PM, Vicente J. Botet Escriba
> <vicente.botet_at_[hidden]> wrote:
>> Le 13/01/13 16:16, Andrey Semashev a écrit :
>>>
>>> once_flag must always be statically initialized, that's the key feature
>>> that
>>> allows call_once to be thread-safe. In C++03 this can only be guaranteed
>>> for
>>> POD types. C++11 also includes constant initialization into static
>>> initialization (i.e. when objects are initialized with constexpr
>>> constructors). All other objects are initialized during dynamic
>>> initialization, which is not thread-safe.
>>>
>> Thanks for clarifications, I understand your concern. The non-POD once_flag
>> implementation can not be used for static once_flag instances in a thread
>> safe mode.
>> Just for curiosity, from where comes the restriction that once_flag must
>> always be statically initialized? Could you point me to the standard?
> The standard only contains once_flag since C++11, and as you quoted
> its definition, it uses constexpr default constructor. The fact that
> it results in static initialization is outlined in 3.6.2/2.
>
> As for C++03, I cannot point you to a particular paper that defines
> once_flag; this is a matter of each particular implementation.
> However, it is known that dynamic initialization is just as unsafe in
> C++03 as it is in C++11, even more unsafe WRT function-local statics.
> In 3.6.2/1 in C++03 it is stated that "Objects of POD types (3.9) with
> static storage duration initialized with constant expressions (5.19)
> shall be initialized before any dynamic initialization takes place.",
> so one (if not the only one) way to implement once_flag safely is to
> make it a POD type.
>
> One thing still concerns me though with the C++11 code. The standard
> specifies that once_flag as non-copyable. This is quite reasonable
> when once_flag is used as described by the standard. OTOH,
> Boost.Thread supports the following code for backward compatibility:
>
>    static once_flag flag = BOOST_ONCE_INIT;
>
> which expands to:
>
>    static once_flag flag = once_flag();
> Technically, this invokes the default constructor and then the copy
> constructor. Compilers may be smart enough to optimize the copy away,
> which explains why it compiles. But I'm not quite sure this is still a
> constant initialization, especially considering the bug in clang (it
> defines implicit constructor without constexpr). To be on the safe
> side I would probably define a special explicit initializing
> constructor marked with constexpr so that the initialization could
> look like this:
>
>    static once_flag flag = {0};
>
>
Well, the definition removes the copy constructor and the assignment

struct once_flag
{
BOOST_THREAD_NO_COPYABLE(once_flag)

After adding

static boost::once_flag once2 = BOOST_ONCE_INIT;

to example/once.cpp

and I'm seen the following kind of error on a lot of C++11 compilers

darwin.compile.c++ 
../../../bin.v2/libs/thread/test/ex_once2.test/darwin-4.7.0x/debug/threading-multi/once.o
../example/once.cpp:16:33: error: use of deleted function 
‘boost::once_flag::once_flag(const boost::once_flag&)’
In file included from ../../../boost/thread/once.hpp:19:0,
from ../example/once.cpp:10:
../../../boost/thread/pthread/once_atomic.hpp:37:5: error: declared here

or

clang-darwin.compile.c++ 
../../../bin.v2/libs/thread/test/ex_once2_lib.test/clang-darwin-3.2xl/debug/threading-multi/once.o
../example/once.cpp:16:25: error: call to deleted constructor of 
'boost::once_flag'
static boost::once_flag once2 = BOOST_ONCE_INIT;
^ ~~~~~~~~~~~~~~~
../../../boost/thread/pthread/once_atomic.hpp:37:30: note: function has 
been explicitly marked deleted here
BOOST_THREAD_NO_COPYABLE(once_flag)
^
../../../boost/thread/detail/delete.hpp:42:35: note: expanded from macro 
'BOOST_THREAD_NO_COPYABLE'
BOOST_THREAD_DELETE_COPY_CTOR(CLASS) \
^
../../../boost/thread/detail/delete.hpp:20:7: note: expanded from macro 
'BOOST_THREAD_DELETE_COPY_CTOR'
CLASS(CLASS const&) = delete; \
^

Maybe it is worth to add a portable extension to the C++11 to ensure 
compatibility.

BTW, couldn't the lock scope be reduced as follows

BOOST_THREAD_DECL void commit_once_region(once_flag& flag) BOOST_NOEXCEPT
{
atomic_type& f = reinterpret_cast< atomic_type& >(flag.storage);
{ // ++++++++
pthread::pthread_mutex_scoped_lock lk(&once_mutex);
f.store(initialized, memory_order_release);
} // ++++++++
BOOST_VERIFY(!pthread_cond_broadcast(&once_cv));
}

Best,
Vicente