2007-10-10 Jason Merrill * libsupc++/guard.cc (struct mutex_wrapper): Move into anonymous namespace. 2007-10-09 Zhou Drangon PR libstdc++/33682 * libsupc++/guard.cc: Make single conditional variable implementation dependent to __GTHREAD_HAS_COND. 2007-10-05 Hans-Peter Nilsson * gthr-single.h: Revert last change. 2007-10-04 Doug Kwan * gthr-posix.h (__gthread_cond_broadcast, __gthread_cond_wait, __gthread_cond_wait_recursive): Add to extend interface for POSIX conditional variables. (__GTHREAD_HAS_COND): Macro defined to signify support of conditional variables. * gthr-posix95.h (__gthread_cond_broadcast, __gthread_cond_wait, __gthread_cond_wait_recursive): Add to extend interface for POSIX conditional variables. (__GTHREAD_HAS_COND): Macro defined to signify support of conditional variables. * gthr-single.h (__gthread_cond_broadcast, __gthread_cond_wait, __gthread_cond_wait_recursive): Add to extend interface for POSIX conditional variables. * gthr.h: Update comments to document new interface. * include/ext/concurrent.h (class __mutex, class __recursive_mutex): Add new method gthread_mutex to access inner gthread mutex. [__GTHREAD_HAS_COND] (class __concurrence_broadcast_error, class __concurrence_wait_error, class __cond): Add. * libsupc++/guard.cc (recursive_push, recursive_pop): Delete. (init_in_progress_flag, set_init_in_progress_flag): Add to replace recursive_push and recursive_pop. (throw_recursive_init_exception): Add. (acquire, __cxa_guard_acquire, __cxa_guard_abort and __cxa_guard_release): [__GTHREAD_HAS_COND] Use a conditional for synchronization of static variable initialization. The global mutex is only held briefly when guards are accessed. [!__GTHREAD_HAS_COND] Fall back to the old code, which deadlocks. * testsuite/thread/guard.cc: Add new test. It deadlocks with the old locking code in libstdc++-v3/libsup++/guard.cc. --- gcc/gthr.h (revision 129029) +++ gcc/gthr.h (revision 129030) @@ -81,6 +81,24 @@ Software Foundation, 51 Franklin Street, int __gthread_recursive_mutex_trylock (__gthread_recursive_mutex_t *mutex); int __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *mutex); + The following are supported in POSIX threads only. They are required to + fix a deadlock in static initialization inside libsupc++. The header file + gthr-posix.h defines a symbol __GTHREAD_HAS_COND to signify that these extra + features are supported. + + Types: + __gthread_cond_t + + Macros: + __GTHREAD_COND_INIT + __GTHREAD_COND_INIT_FUNCTION + + Interface: + int __gthread_cond_broadcast (__gthread_cond_t *cond); + int __gthread_cond_wait (__gthread_cond_t *cond, __gthread_mutex_t *mutex); + int __gthread_cond_wait_recursive (__gthread_cond_t *cond, + __gthread_recursive_mutex_t *mutex); + All functions returning int should return zero on success or the error number. If the operation is not supported, -1 is returned. --- gcc/gthr-posix.h (revision 129029) +++ gcc/gthr-posix.h (revision 129030) @@ -47,6 +47,11 @@ typedef pthread_key_t __gthread_key_t; typedef pthread_once_t __gthread_once_t; typedef pthread_mutex_t __gthread_mutex_t; typedef pthread_mutex_t __gthread_recursive_mutex_t; +typedef pthread_cond_t __gthread_cond_t; + +/* POSIX like conditional variables are supported. Please look at comments + in gthr.h for details. */ +#define __GTHREAD_HAS_COND 1 #define __GTHREAD_MUTEX_INIT PTHREAD_MUTEX_INITIALIZER #define __GTHREAD_ONCE_INIT PTHREAD_ONCE_INIT @@ -57,6 +62,7 @@ typedef pthread_mutex_t __gthread_recurs #else #define __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION __gthread_recursive_mutex_init_function #endif +#define __GTHREAD_COND_INIT PTHREAD_COND_INITIALIZER #if SUPPORTS_WEAK && GTHREAD_USE_WEAK # ifndef __gthrw_pragma @@ -88,6 +94,8 @@ __gthrw3(pthread_mutex_lock) __gthrw3(pthread_mutex_trylock) __gthrw3(pthread_mutex_unlock) __gthrw3(pthread_mutex_init) +__gthrw3(pthread_cond_broadcast) +__gthrw3(pthread_cond_wait) #else __gthrw(pthread_once) __gthrw(pthread_getspecific) @@ -98,6 +106,8 @@ __gthrw(pthread_mutex_lock) __gthrw(pthread_mutex_trylock) __gthrw(pthread_mutex_unlock) __gthrw(pthread_mutex_init) +__gthrw(pthread_cond_broadcast) +__gthrw(pthread_cond_wait) #endif __gthrw(pthread_key_create) @@ -110,20 +120,16 @@ __gthrw(pthread_mutexattr_destroy) #if defined(_LIBOBJC) || defined(_LIBOBJC_WEAK) /* Objective-C. */ #if defined(__osf__) && defined(_PTHREAD_USE_MANGLED_NAMES_) -__gthrw3(pthread_cond_broadcast) __gthrw3(pthread_cond_destroy) __gthrw3(pthread_cond_init) __gthrw3(pthread_cond_signal) -__gthrw3(pthread_cond_wait) __gthrw3(pthread_exit) __gthrw3(pthread_mutex_destroy) __gthrw3(pthread_self) #else -__gthrw(pthread_cond_broadcast) __gthrw(pthread_cond_destroy) __gthrw(pthread_cond_init) __gthrw(pthread_cond_signal) -__gthrw(pthread_cond_wait) __gthrw(pthread_exit) __gthrw(pthread_mutex_destroy) __gthrw(pthread_self) @@ -737,6 +743,25 @@ __gthread_recursive_mutex_unlock (__gthr return __gthread_mutex_unlock (mutex); } +static inline int +__gthread_cond_broadcast (__gthread_cond_t *cond) +{ + return __gthrw_(pthread_cond_broadcast) (cond); +} + +static inline int +__gthread_cond_wait (__gthread_cond_t *cond, __gthread_mutex_t *mutex) +{ + return __gthrw_(pthread_cond_wait) (cond, mutex); +} + +static inline int +__gthread_cond_wait_recursive (__gthread_cond_t *cond, + __gthread_recursive_mutex_t *mutex) +{ + return __gthread_cond_wait (cond, mutex); +} + #endif /* _LIBOBJC */ #endif /* ! GCC_GTHR_POSIX_H */ --- gcc/gthr-posix95.h (revision 129029) +++ gcc/gthr-posix95.h (revision 129030) @@ -45,6 +45,11 @@ Software Foundation, 51 Franklin Street, typedef pthread_key_t __gthread_key_t; typedef pthread_once_t __gthread_once_t; typedef pthread_mutex_t __gthread_mutex_t; +typedef pthread_cond_t __gthread_cond_t; + +/* POSIX like conditional variables are supported. Please look at comments + in gthr.h for details. */ +#define __GTHREAD_HAS_COND 1 typedef struct { long depth; @@ -55,6 +60,7 @@ typedef struct { #define __GTHREAD_MUTEX_INIT PTHREAD_MUTEX_INITIALIZER #define __GTHREAD_ONCE_INIT PTHREAD_ONCE_INIT #define __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION __gthread_recursive_mutex_init_function +#define __GTHREAD_COND_INIT PTHREAD_COND_INITIALIZER #if SUPPORTS_WEAK && GTHREAD_USE_WEAK # define __gthrw(name) \ @@ -81,14 +87,14 @@ __gthrw(pthread_mutexattr_init) __gthrw(pthread_mutexattr_destroy) __gthrw(pthread_mutex_init) +__gthrw(pthread_cond_broadcast) +__gthrw(pthread_cond_wait) #if defined(_LIBOBJC) || defined(_LIBOBJC_WEAK) /* Objective-C. */ -__gthrw(pthread_cond_broadcast) __gthrw(pthread_cond_destroy) __gthrw(pthread_cond_init) __gthrw(pthread_cond_signal) -__gthrw(pthread_cond_wait) __gthrw(pthread_exit) __gthrw(pthread_mutex_destroy) #ifdef _POSIX_PRIORITY_SCHEDULING @@ -719,6 +725,25 @@ __gthread_recursive_mutex_unlock (__gthr return 0; } +static inline int +__gthread_cond_broadcast (__gthread_cond_t *cond) +{ + return __gthrw_(pthread_cond_broadcast) (cond); +} + +static inline int +__gthread_cond_wait (__gthread_cond_t *cond, __gthread_mutex_t *mutex) +{ + return __gthrw_(pthread_cond_wait) (cond, mutex); +} + +static inline int +__gthread_cond_wait_recursive (__gthread_cond_t *cond, + __gthread_recursive_mutex_t *mutex) +{ + return __gthrw_(pthread_cond_wait) (cond, mutex->actual); +} + #endif /* _LIBOBJC */ #endif /* ! GCC_GTHR_POSIX_H */ --- libstdc++-v3/libsupc++/guard.cc (revision 129029) +++ libstdc++-v3/libsupc++/guard.cc (revision 129218) @@ -34,6 +34,9 @@ #include #include #include +#if defined(__GTHREAD_HAS_COND) && !defined(__GTHREAD_COND_INIT) +#undef __GTHREAD_HAS_COND +#endif // The IA64/generic ABI uses the first byte of the guard variable. // The ARM EABI uses the least significant bit. @@ -46,7 +49,7 @@ namespace // This is a static class--the need for a static initialization function // to pass to __gthread_once precludes creating multiple instances, though // I suppose you could achieve the same effect with a template. - class static_mutex + struct static_mutex { static __gthread_recursive_mutex_t mutex; @@ -85,8 +88,30 @@ namespace { __gthread_recursive_mutex_unlock (&mutex); } + + // Simple wrapper for exception safety. + struct mutex_wrapper + { + bool unlock; + mutex_wrapper() : unlock(true) + { static_mutex::lock(); } + + ~mutex_wrapper() + { + if (unlock) + static_mutex::unlock(); + } + }; } +#ifdef __GTHREAD_HAS_COND +namespace +{ + // A single conditional variable controlling all static initializations. + static __gthread_cond_t static_cond = __GTHREAD_COND_INIT; +} +#endif + #ifndef _GLIBCXX_GUARD_TEST_AND_ACQUIRE inline bool __test_and_acquire (__cxxabiv1::__guard *g) @@ -135,19 +160,43 @@ namespace __gnu_cxx recursive_init::~recursive_init() throw() { } } +// +// Here are C++ run-time routines for guarded initiailization of static +// variables. There are 4 scenarios under which these routines are called: +// +// 1. Threads not supported (__GTHREADS not defined) +// 2. Threads are supported but not enabled at run-time. +// 3. Threads enabled at run-time but __gthreads_* are not fully POSIX. +// 4. Threads enabled at run-time and __gthreads_* support all POSIX threads +// primitives we need here. +// +// The old code supported scenarios 1-3 but was broken since it used a global +// mutex for all threads and had the mutex locked during the whole duration of +// initlization of a guarded static variable. The following created a dead-lock +// with the old code. +// +// Thread 1 acquires the global mutex. +// Thread 1 starts initializing static variable. +// Thread 1 creates thread 2 during initialization. +// Thread 2 attempts to acuqire mutex to initialize another variable. +// Thread 2 blocks since thread 1 is locking the mutex. +// Thread 1 waits for result from thread 2 and also blocks. A deadlock. +// +// The new code here can handle this situation and thus is more robust. Howere, +// we need to use the POSIX thread conditional variable, which is not supported +// in all platforms, notably older versions of Microsoft Windows. The gthr*.h +// headers define a symbol __GTHREAD_HAS_COND for platforms that support POSIX +// like conditional variables. For platforms that do not support conditional +// variables, we need to fall back to the old code. namespace __cxxabiv1 { static inline int - recursion_push (__guard* g) - { - return ((char *)g)[1]++; - } + init_in_progress_flag(__guard* g) + { return ((char *)g)[1]; } static inline void - recursion_pop (__guard* g) - { - --((char *)g)[1]; - } + set_init_in_progress_flag(__guard* g, int v) + { ((char *)g)[1] = v; } static int acquire_1 (__guard *g) @@ -155,7 +204,7 @@ namespace __cxxabiv1 if (_GLIBCXX_GUARD_TEST (g)) return 0; - if (recursion_push (g)) + if (init_in_progress_flag (g)) { #ifdef __EXCEPTIONS throw __gnu_cxx::recursive_init(); @@ -164,6 +213,7 @@ namespace __cxxabiv1 __builtin_trap (); #endif } + set_init_in_progress_flag(g, 1); return 1; } @@ -179,28 +229,40 @@ namespace __cxxabiv1 if (__gthread_active_p ()) { - // Simple wrapper for exception safety. - struct mutex_wrapper - { - bool unlock; - mutex_wrapper (): unlock(true) - { - static_mutex::lock (); - } - ~mutex_wrapper () - { - if (unlock) - static_mutex::unlock (); - } - } mw; + mutex_wrapper mw; - if (acquire_1 (g)) - { - mw.unlock = false; - return 1; + while (1) // When this loop is executing, mutex is locked. + { +#ifdef __GTHREAD_HAS_COND + // The static is allready initialized. + if (_GLIBCXX_GUARD_TEST(g)) + return 0; // The mutex will be unlocked via wrapper + + if (init_in_progress_flag(g)) + { + // The guarded static is currently being initialized by + // another thread, so we release mutex and wait for the + // conditional variable. We will lock the mutex again after + // this. + __gthread_cond_wait_recursive(&static_cond, + &static_mutex::mutex); + } + else + { + set_init_in_progress_flag(g, 1); + return 1; // The mutex will be unlocked via wrapper. + } +#else + // This provides compatibility with older systems not supporting + // POSIX like conditional variables. + if (acquire_1 (g)) + { + mw.unlock = false; + return 1; // The mutex still locked. + } + return 0; // The mutex will be unlocked via wrapper. +#endif } - - return 0; } #endif @@ -210,8 +272,24 @@ namespace __cxxabiv1 extern "C" void __cxa_guard_abort (__guard *g) { - recursion_pop (g); -#ifdef __GTHREADS +#ifdef __GTHREAD_HAS_COND + if (__gthread_active_p()) + { + mutex_wrapper mw; + + set_init_in_progress_flag(g, 0); + + // If we abort, we still need to wake up all other threads waiting for + // the conditional variable. + __gthread_cond_broadcast(&static_cond); + return; + } +#endif + + set_init_in_progress_flag(g, 0); +#if defined(__GTHREADS) && !defined(__GTHREAD_HAS_COND) + // This provides compatibility with older systems not supporting POSIX like + // conditional variables. if (__gthread_active_p ()) static_mutex::unlock (); #endif @@ -220,11 +298,27 @@ namespace __cxxabiv1 extern "C" void __cxa_guard_release (__guard *g) { - recursion_pop (g); +#ifdef __GTHREAD_HAS_COND + if (__gthread_active_p()) + { + mutex_wrapper mw; + + set_init_in_progress_flag(g, 0); + _GLIBCXX_GUARD_SET_AND_RELEASE(g); + + __gthread_cond_broadcast(&static_cond); + return; + } +#endif + + set_init_in_progress_flag(g, 0); _GLIBCXX_GUARD_SET_AND_RELEASE (g); -#ifdef __GTHREADS - if (__gthread_active_p ()) - static_mutex::unlock (); + +#if defined(__GTHREADS) && !defined(__GTHREAD_HAS_COND) + // This provides compatibility with older systems not supporting POSIX like + // conditional variables. + if (__gthread_active_p()) + static_mutex::unlock(); #endif } } --- libstdc++-v3/testsuite/thread/guard.cc (revision 0) +++ libstdc++-v3/testsuite/thread/guard.cc (revision 129030) @@ -0,0 +1,67 @@ +// +// Copyright (C) 2007 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 2, or (at your option) +// any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING. If not, write to the Free +// Software Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, +// USA. + +// { dg-do run { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-solaris* *-*-cygwin *-*-darwin* alpha*-*-osf* } } +// { dg-options "-pthread" { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-solaris* *-*-darwin* alpha*-*-osf* } } + +#include +#include + +// This used to deadlock with the old libstdc++ because there is only one +// global mutex guarding initialization of statics and it is held during by +// the initializer thread of a static until the variable is completely +// initialized. If the initializer thread creates and waits for another thread +// which also initializes a static variable, there will be a deadlock because +// the first thread is holding the mutex and waiting for the second thread, +// which is blocked when it is acquiring the mutex. + +int +get_bar (void) +{ + return 1; +} + +void* +do_something (void *arg) +{ + static int bar = get_bar (); + return NULL; +} + +int +get_foo (void) +{ + int status; + pthread_t new_thread; + + if (pthread_create (&new_thread, NULL, do_something, NULL) != 0) + std::abort (); + + if (pthread_join (new_thread, NULL) != 0) + std::abort (); + + return 1; +} + +int +main (int argc, char **argv) +{ + static int foo = get_foo (); + return 0; +}