boost::call_once() is unreliable on some platforms

[Matthew Dempsky <matthew@…>]

boost::call_once() is an implementation of Mike Burrows's fast_pthread_once() algorithm, as described in ​http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2660.htm. In the correctness proof, there is a requirement that loads and stores of the epoch value are atomic (i.e., do not exhibit word tearing). In Mike's example implementation, the sig_atomic_t type is used, which is required by the C standard to support atomic loads and stores.

However, in the boost::call_once() implementation, the epoch value is defined as uintmax_t, which has no such guarantee, and in practice is not atomic on some architectures. E.g., on OpenBSD/i386, uintmax_t is a 64-bit type and assignments to a 64-bit memory address must be split into two (non-atomic) store instructions.

Therefore, thread/pthread/once.hpp should be changed to use a type that is guaranteed to support atomic loads and stores instead of uintmax_t. Additionally, since once_flag::epoch is accessed by multiple threads without any synchronization, it should be marked volatile.

(Alternatively, the new C++0x atomic operations library appears suitable for this use as well.)

[viboes]

I would like to make it more reliable, but I don't know on which platforms uintmax_t is unreliable and which type will be reliable for these platforms. Is sig_atomic_t available and reliable on these platforms? Could some one help on this?

[viboes]

Does sig_atomic_t works for you?

[viboes]

Committed in trunk revision [80078].

[viboes]

Committed in trunk revision [80082].

[fhess]

When SIG_ATOMIC_MAX is defined (that is, when I compile on Linux with g++ 4.4.5 and -std=c++0x) call_once does not work. It never calls my function at all. I debugged it, it happens because in thread/pthread/once.hpp in the call_once method it compares epoch and this_thread_epoch. epoch is set to 0 and this_thread_epoch is set to -1 so the comparison is false and call once does nothing. It seems the problem is due to uintmax_atomic_t being defined as a sig_atomic_t which turns out to be a signed rather than unsigned type.

[fhess]

Some more info, it seems the problem is due to SIG_ATOMIC_MAX being defined in my code when I include once.hpp, but it is not defined in once.cpp when the libboost_thread is compiled. By default, cstdint doesn't define SIG_ATOMIC_MAX when compiling in c++ mode, unless you define __STDC_LIMIT_MACROS before stdint.h is included.

[viboes]

I will add it

Index: pthread/once.hpp
===================================================================
--- pthread/once.hpp	(revision 80450)
+++ pthread/once.hpp	(working copy)
@@ -17,6 +17,8 @@
 #include <boost/thread/pthread/pthread_mutex_scoped_lock.hpp>
 #include <boost/cstdint.hpp>
 #include <boost/thread/detail/delete.hpp>
+// Force SIG_ATOMIC_MAX tobe defined
+#define __STDC_LIMIT_MACROS
 #include <csignal>
 
 #include <boost/config/abi_prefix.hpp>

[viboes]

Committed in trunk revision 80466.

[Frank Mori Hess]

SIG_ATOMIC_MAX actually gets defined by stdint.h/cstdint not csignal so it seems like your define should be earlier. Actually, even then there's still a problem since the user might include stdint.h before they include the once.hpp, without having __STD_LIMIT_MACROS defined. In that case, SIG_ATOMIC_MAX won't be defined in their code but it will be defined in the thread library, leading to incompatible types again.

[viboes]

Replying to fmhess:

SIG_ATOMIC_MAX actually gets defined by stdint.h/cstdint not csignal so it seems like your define should be earlier. Actually, even then there's still a problem since the user might include stdint.h before they include the once.hpp, without having __STD_LIMIT_MACROS defined. In that case, SIG_ATOMIC_MAX won't be defined in their code but it will be defined in the thread library, leading to incompatible types again.

It seems then that I can not relay on whether SIG_ATOMIC_MAX is defined or not, but I need a value to mean not-a-valid value. Have a suggestion?

[Frank Mori Hess]

Replying to viboes:

It seems then that I can not relay on whether SIG_ATOMIC_MAX is defined or not, but I need a value to mean not-a-valid value. Have a suggestion?

How about using a boost::atomic<unsigned long> for the epoch?

[viboes]

Replying to fmhess:

Replying to viboes:

It seems then that I can not relay on whether SIG_ATOMIC_MAX is defined or not, but I need a value to mean not-a-valid value. Have a suggestion?

How about using a boost::atomic<unsigned long> for the epoch?

I will try to use it when released. I can not depend on a non released library.

[kab@…]

Replying to fhess:

When SIG_ATOMIC_MAX is defined ... call_once does not work. ... It seems the problem is due to uintmax_atomic_t being defined as a sig_atomic_t which turns out to be a signed rather than unsigned type.

This problem with using sig_atomic_t has been reported separately: ​https://svn.boost.org/trac/boost/ticket/7499 Too bad I didn't know about this sooner.

[Andrey Semashev]

The patch ports POSIX implementation of call_once to Boost.Atomic.

[viboes]

Committed in trunk revision [82513].

[viboes]

merged from trunk [82838]