PR 5432: Thread safety patches

Nathan Myers ncm-nospam@cantrip.org
Sat Jan 19 12:35:00 GMT 2002


This looks like a very good start.  

A question for the group: In

   static _Atomic_word top = 4; 

below, is the initialization guaranteed to happen at link time,
rather than on function entry time, on all architectures?  

Note that it may (come to) be a struct on some.  If that happens, then 
if its constructor is declared explicit (as it should be), the above 
will fail, which is good, because the above would not be thread-safe 
if it involved a constructor call that would necessarily happen at 
function entry.  Probably a comment to that effect would be advisable.

To make iostreams thread-safe, we also need for the sentry constructor 
and destructor to claim/release a counting mutex.

Nathan Myers
ncm at cantrip dot org

On Sat, Jan 19, 2002 at 01:20:39PM -0500, Craig Rodrigues wrote:
> 
> The following patch and example program was submitted in PR 5432:
> http://gcc.gnu.org/cgi-bin/gnatsweb.pl?database=gcc&pr=5432&cmd=view
> 
> Is the patch OK for commit?  It it OK for 3.0-branch?
> 
> main.cxx
> ------------------------------------------------------------------------------
> #include <pthread.h>
> #include <sstream>
> 
> const size_t num_threads = 4;
> 
> extern "C" {
>     static void*
>     threadFunc(void*)
>     {
>         for (size_t i = 0; i < 10000; ++i) {
>             std::stringstream str;
>         }
> 
>         pthread_exit(0);
> 
>         return(0);
>     }
> }
> 
> int
> main()
> {
>     pthread_t threads[num_threads];
> 
> #if !defined(__linux__)
>     thr_setconcurrency(num_threads);
> #endif
> 
>     for (size_t i = 0; i < num_threads; ++i) {
>         pthread_create(&(threads[i]), 0, threadFunc, 0);
>     }
> 
>     for (size_t i = 0; i < num_threads; ++i) {
>         pthread_join(threads[i], 0);
>     }
> 
>     pthread_exit(0);
> 
>     return (0);
> }
> ------------------------------------------------------------------------------
> 
> 
> 
> 
> 2002-01-19  Craig Rodrigues  <rodrigc@gcc.gnu.org>
> 
> 	PR libstdc++/5432
> 	* include/bits/ios_base.h (_Callback_list): Use
> 	atomic types and functions for reference counting.
> 	* include/bits/localefwd.h (locale::_Impl): Same.
> 	* src/ios.cc (ios_base::xalloc): Same.
> 	* src/locale.cc (locale::classic, locale::facet::_M_add_reference,
> 	locale::facet::_M_remove_reference): Same.
> 
> 
> Index: include/bits/ios_base.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/libstdc++-v3/include/bits/ios_base.h,v
> retrieving revision 1.8.2.3
> diff -u -r1.8.2.3 ios_base.h
> --- ios_base.h	2001/06/06 01:39:00	1.8.2.3
> +++ ios_base.h	2002/01/17 12:53:38
> @@ -36,6 +36,8 @@
>  
>  #pragma GCC system_header
>  
> +#include <bits/atomicity.h>
> +
>  namespace std
>  {
>    // The following definitions of bitmask types are enums, not ints,
> @@ -239,17 +241,17 @@
>        _Callback_list* 		_M_next;
>        ios_base::event_callback 	_M_fn;
>        int 			_M_index;
> -      int 			_M_refcount;  // 0 means one reference.
> +      _Atomic_word		_M_refcount;  // 0 means one reference.
>      
>        _Callback_list(ios_base::event_callback __fn, int __index, 
>  		     _Callback_list* __cb)
>        : _M_next(__cb), _M_fn(__fn), _M_index(__index), _M_refcount(0) { }
>        
>        void 
> -      _M_add_reference() { ++_M_refcount; } // XXX MT
> +      _M_add_reference() { __atomic_add(&_M_refcount, 1); } // XXX MT
>        
>        int 
> -      _M_remove_reference() { return _M_refcount--; }  // 0 => OK to delete
> +      _M_remove_reference() { return __exchange_and_add(&_M_refcount, -1); }  // 0 => OK to delete
>      };
>  
>       _Callback_list*  	_M_callbacks;
> Index: include/bits/localefwd.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/libstdc++-v3/include/bits/localefwd.h,v
> retrieving revision 1.11.2.3
> diff -u -r1.11.2.3 localefwd.h
> --- localefwd.h	2001/05/14 19:49:03	1.11.2.3
> +++ localefwd.h	2002/01/17 12:53:38
> @@ -43,6 +43,8 @@
>  #include <bits/std_cctype.h>	// For isspace, etc.
>  #include <bits/functexcept.h>
>  
> +#include <bits/atomicity.h>
> +
>  namespace std
>  {
>    // NB: Don't instantiate required wchar_t facets if no wchar_t support.
> @@ -307,7 +309,7 @@
>  
>    private:
>      // Data Members.
> -    size_t 				_M_references;
> +    _Atomic_word			_M_references;
>      __vec_facet* 			_M_facets;
>      string 				_M_names[_S_num_categories];
>      __c_locale				_M_c_locale;
> @@ -321,12 +323,12 @@
>  
>      inline void 
>      _M_add_reference() throw()
> -    { ++_M_references; }  // XXX MT
> +    { __atomic_add(&_M_references, 1); }  // XXX MT
>  
>      inline void 
>      _M_remove_reference() throw()
>      {
> -      if (_M_references-- == 0)  // XXX MT
> +      if (__exchange_and_add(&_M_references, -1) == 0)  // XXX MT
>  	{
>  	  try 
>  	    { delete this; } 
> @@ -394,7 +396,7 @@
>      _S_destroy_c_locale(__c_locale& __cloc);
>  
>    private:
> -    size_t _M_references;
> +    _Atomic_word _M_references;
>  
>      void 
>      _M_add_reference() throw();
> @@ -428,7 +430,7 @@
>      mutable size_t 	_M_index;
>  
>      // Last id number assigned
> -    static size_t 	_S_highwater;   
> +    static _Atomic_word 	_S_highwater;   
>  
>      void 
>      operator=(const id&);  // not defined
> Index: src/ios.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libstdc++-v3/src/ios.cc,v
> retrieving revision 1.14.4.2
> diff -u -r1.14.4.2 ios.cc
> --- ios.cc	2001/06/06 01:39:01	1.14.4.2
> +++ ios.cc	2002/01/17 12:53:38
> @@ -36,6 +36,8 @@
>  #include <bits/std_istream.h>
>  #include <bits/std_fstream.h>
>  
> +#include <bits/atomicity.h>
> +
>  namespace std 
>  {
>    // Extern declarations for global objects in src/globals.cc.
> @@ -216,8 +218,8 @@
>    {
>      // XXX MT
>      // XXX should be a symbol. (Reserve 0..3 for builtins.)
> -    static int top = 4; 
> -    return top++;
> +    static _Atomic_word top = 4; 
> +    return __exchange_and_add(&top, 1);
>    }
>  
>    // 27.4.2.5  iword/pword storage
> Index: src/locale.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libstdc++-v3/src/locale.cc,v
> retrieving revision 1.28.2.5
> diff -u -r1.28.2.5 locale.cc
> --- locale.cc	2001/12/11 08:01:24	1.28.2.5
> +++ locale.cc	2002/01/17 12:53:39
> @@ -41,6 +41,8 @@
>  # include <bits/std_cwctype.h>     // for towupper, etc.
>  #endif
>  
> +#include <bits/atomicity.h>
> +
>  namespace std 
>  {
>    // Definitions for static const data members of locale.
> @@ -68,7 +70,7 @@
>  #endif
>  
>    // Definitions for static const data members of locale::id
> -  size_t locale::id::_S_highwater;  // init'd to 0 by linker
> +  _Atomic_word locale::id::_S_highwater;  // init'd to 0 by linker
>  
>    // Definitions for static const data members of locale::_Impl
>    const locale::id* const
> @@ -445,6 +447,9 @@
>    locale::classic()
>    {
>      static locale* __classic_locale;
> +    static _STL_mutex_lock __lock __STL_MUTEX_INITIALIZER;
> +    _STL_auto_lock __auto(__lock);
> +
>      // XXX MT
>      if (!_S_classic)
>        {
> @@ -523,15 +528,13 @@
>    void  
>    locale::facet::
>    _M_add_reference() throw()
> -  { ++_M_references; }                     // XXX MT
> +  { __atomic_add(&_M_references, 1); }                     // XXX MT
>  
>    void  
>    locale::facet::
>    _M_remove_reference() throw()
>    {
> -    if (_M_references)
> -      --_M_references;
> -    else
> +    if (__exchange_and_add(&_M_references, -1) == 0)
>        {
>          try 
>  	  { delete this; }  // XXX MT
> Index: src/localename.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libstdc++-v3/src/localename.cc,v
> retrieving revision 1.12.2.2
> diff -u -r1.12.2.2 localename.cc
> --- localename.cc	2001/05/14 19:49:15	1.12.2.2
> +++ localename.cc	2002/01/17 12:53:39
> @@ -173,7 +173,7 @@
>        {
>  	size_t& __index = __idp->_M_index;
>  	if (!__index)
> -	  __index = ++locale::id::_S_highwater;  // XXX MT
> +	  __index = 1 + __exchange_and_add(&locale::id::_S_highwater, 1);  // XXX MT
>  	
>  	if (__index >= _M_facets->size())
>  	  _M_facets->resize(__index + 1, 0);  // might throw
> 	
> -- 
> Craig Rodrigues        
> http://www.gis.net/~craigr    
> rodrigc@mediaone.net          



More information about the Libstdc++ mailing list