diff mbox

PR 53889: Add __gthread_recursive_mutex_destroy

Message ID CAH6eHdS7pZ+bnhS35z6YEdGiAjuQA8VugQM-8ZDf3Xb9zdD6iQ@mail.gmail.com
State New
Headers show

Commit Message

Jonathan Wakely Sept. 30, 2012, 6:41 p.m. UTC 
There is no __gthread_recursive_mutex_destroy function in the gthreads API.

Trying to use __gthread_mutex_destroy fails to compile on platforms
where the mutex
types are different. To avoid resource leaks libstdc++ needs to hack
around the missing function with overloaded functions and SFINAE
tricks to detect how a recursive mutex can be destroyed.

This patch extends the gthreads API to include
__gthread_recursive_mutex_destroy, defining it for each gthread model,
and removing the hacks from libstdc++.

libgcc:

        PR other/53889
        * gthr.h (__gthread_recursive_mutex_destroy): Document new required
        function.
        * gthr-posix.h (__gthread_recursive_mutex_destroy): Define.
        * gthr-single.h (__gthread_recursive_mutex_destroy): Likewise.
        * config/gthr-rtems.h (__gthread_recursive_mutex_destroy): Likewise.
        * config/gthr-vxworks.h (__gthread_recursive_mutex_destroy): Likewise.
        * config/i386/gthr-win32.h (__gthread_recursive_mutex_destroy):
        Likewise.
        * config/mips/gthr-mipssde.h (__gthread_recursive_mutex_destroy):
        Likewise.
        * config/pa/gthr-dce.h (__gthread_recursive_mutex_destroy): Likewise.
        * config/s390/gthr-tpf.h (__gthread_recursive_mutex_destroy): Likewise.

libstdc++-v3:

        PR other/53889
        * include/std/mutex (__recursive_mutex_base::~__recursive_mutex_base):
        Use __gthread_recursive_mutex_destroy.
        (__recursive_mutex_base::_S_destroy): Remove.
        (__recursive_mutex_base::_S_destroy_win32): Likewise.
        * include/ext/concurrence.h (__recursive_mutex::~__recursive_mutex):
        Use __gthread_recursive_mutex_destroy.
        (__recursive_mutex::_S_destroy): Remove.
        (__recursive_mutex::_S_destroy_win32): Likewise.


Tested x86_64-linux.

Are the libgcc parts OK for trunk?
commit 37d75fef68222e92c4b58870dcfeeb3679e3c718
Author: Jonathan Wakely <jwakely.gcc@gmail.com>
Date:   Sun Sep 30 19:00:51 2012 +0100

    libgcc:
    
    	PR other/53889
    	* gthr.h (__gthread_recursive_mutex_destroy): Document new required
    	function.
    	* gthr-posix.h (__gthread_recursive_mutex_destroy): Define.
    	* gthr-single.h (__gthread_recursive_mutex_destroy): Likewise.
    	* config/gthr-rtems.h (__gthread_recursive_mutex_destroy): Likewise.
    	* config/gthr-vxworks.h (__gthread_recursive_mutex_destroy): Likewise.
    	* config/i386/gthr-win32.h (__gthread_recursive_mutex_destroy):
    	Likewise.
    	* config/mips/gthr-mipssde.h (__gthread_recursive_mutex_destroy):
    	Likewise.
    	* config/pa/gthr-dce.h (__gthread_recursive_mutex_destroy): Likewise.
    	* config/s390/gthr-tpf.h (__gthread_recursive_mutex_destroy): Likewise.
    
    libstdc++-v3:
    
    	PR other/53889
    	* include/std/mutex (__recursive_mutex_base::~__recursive_mutex_base):
    	Use __gthread_recursive_mutex_destroy.
    	(__recursive_mutex_base::_S_destroy): Remove.
    	(__recursive_mutex_base::_S_destroy_win32): Likewise.
    	* include/ext/concurrence.h (__recursive_mutex::~__recursive_mutex):
    	Use __gthread_recursive_mutex_destroy.
    	(__recursive_mutex::_S_destroy): Remove.
    	(__recursive_mutex::_S_destroy_win32): Likewise.

Comments

Ian Lance Taylor Oct. 1, 2012, 7:22 p.m. UTC | #1 
On Sun, Sep 30, 2012 at 11:41 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> There is no __gthread_recursive_mutex_destroy function in the gthreads API.
>
> Trying to use __gthread_mutex_destroy fails to compile on platforms
> where the mutex
> types are different. To avoid resource leaks libstdc++ needs to hack
> around the missing function with overloaded functions and SFINAE
> tricks to detect how a recursive mutex can be destroyed.
>
> This patch extends the gthreads API to include
> __gthread_recursive_mutex_destroy, defining it for each gthread model,
> and removing the hacks from libstdc++.

> +    return rtems_gxx_mutex_destroy( __mutex );

Space before '(', not space after.

Doing anything else here is going to be painful, but this assumes that
RTEMS uses the same representation for non-recursive and recursive
mutexes.  That is currently true, but it deserves a comment.


> --- a/libgcc/config/i386/gthr-win32.h
> +++ b/libgcc/config/i386/gthr-win32.h
> +static inline void
> +__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex)
> +{
> +  __gthread_mutex_t __mutex2;
> +  __mutex2.sema = mutex->sema;
> +  __gthr_win32_mutex_destroy (&__mutex2);
> +}

I think it would be better to put this in
libgcc/config/i386/gthr-win32.c, like the other functions.  Then you
can just call CloseHandle.

> --- a/libgcc/config/mips/gthr-mipssde.h
> +++ b/libgcc/config/mips/gthr-mipssde.h
>
> +static inline int
> +__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
> +{
> +  return __gthread_mutex_destroy(__mutex);
> +}

Will this even compile?  It doesn't look like it.

Ian
diff mbox

Patch

diff --git a/libgcc/config/gthr-rtems.h b/libgcc/config/gthr-rtems.h
index c5bd522..50bdd9f 100644
--- a/libgcc/config/gthr-rtems.h
+++ b/libgcc/config/gthr-rtems.h
@@ -1,8 +1,7 @@ 
 /* RTEMS threads compatibility routines for libgcc2 and libobjc.
    by: Rosimildo da Silva( rdasilva@connecttel.com ) */
 /* Compile this one with gcc.  */
-/* Copyright (C) 1997, 1999, 2000, 2002, 2003, 2005, 2008, 2009
-   Free Software Foundation, Inc.
+/* Copyright (C) 1997-2012 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -150,6 +149,12 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)
     return rtems_gxx_recursive_mutex_unlock( __mutex );
 }
 
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+    return rtems_gxx_mutex_destroy( __mutex );
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libgcc/config/gthr-vxworks.h b/libgcc/config/gthr-vxworks.h
index 63116c4..b48c5ac 100644
--- a/libgcc/config/gthr-vxworks.h
+++ b/libgcc/config/gthr-vxworks.h
@@ -1,7 +1,6 @@ 
 /* Threads compatibility routines for libgcc2 and libobjc for VxWorks.  */
 /* Compile this one with gcc.  */
-/* Copyright (C) 1997, 1999, 2000, 2008, 2009, 2011
-   Free Software Foundation, Inc.
+/* Copyright (C) 1997-2012 Free Software Foundation, Inc.
    Contributed by Mike Stump <mrs@wrs.com>.
 
 This file is part of GCC.
@@ -111,6 +110,12 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *mutex)
   return __gthread_mutex_unlock (mutex);
 }
 
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+  return __gthread_mutex_destroy (__mutex);
+}
+
 /* pthread_once is complicated enough that it's implemented
    out-of-line.  See config/vxlib.c.  */
 
diff --git a/libgcc/config/i386/gthr-win32.h b/libgcc/config/i386/gthr-win32.h
index 53f8396..cc2cc9b 100644
--- a/libgcc/config/i386/gthr-win32.h
+++ b/libgcc/config/i386/gthr-win32.h
@@ -1,8 +1,7 @@ 
 /* Threads compatibility routines for libgcc2 and libobjc.  */
 /* Compile this one with gcc.  */
 
-/* Copyright (C) 1999, 2000, 2002, 2003, 2004, 2005, 2008, 2009
-   Free Software Foundation, Inc.
+/* Copyright (C) 1999-2012 Free Software Foundation, Inc.
    Contributed by Mumit Khan <khan@xraylith.wisc.edu>.
 
 This file is part of GCC.
@@ -536,6 +535,14 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)
     return 0;
 }
 
+static inline void
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex)
+{
+  __gthread_mutex_t __mutex2;
+  __mutex2.sema = mutex->sema;
+  __gthr_win32_mutex_destroy (&__mutex2);
+}
+
 #else /* ! __GTHREAD_HIDE_WIN32API */
 
 #include <windows.h>
@@ -761,6 +768,12 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)
   return 0;
 }
 
+static inline void
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex)
+{
+  CloseHandle ((HANDLE) mutex->sema);
+}
+
 #endif /*  __GTHREAD_HIDE_WIN32API */
 
 #ifdef __cplusplus
diff --git a/libgcc/config/mips/gthr-mipssde.h b/libgcc/config/mips/gthr-mipssde.h
index 34f9b6c..7c9f537 100644
--- a/libgcc/config/mips/gthr-mipssde.h
+++ b/libgcc/config/mips/gthr-mipssde.h
@@ -1,6 +1,6 @@ 
 /* MIPS SDE threads compatibility routines for libgcc2 and libobjc.  */
 /* Compile this one with gcc.  */
-/* Copyright (C) 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2006-2012 Free Software Foundation, Inc.
    Contributed by Nigel Stephens <nigel@mips.com>
 
 This file is part of GCC.
@@ -223,6 +223,12 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)
   return 0;
 }
 
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+  return __gthread_mutex_destroy(__mutex);
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libgcc/config/pa/gthr-dce.h b/libgcc/config/pa/gthr-dce.h
index d32155a..3ba43a1 100644
--- a/libgcc/config/pa/gthr-dce.h
+++ b/libgcc/config/pa/gthr-dce.h
@@ -1,7 +1,6 @@ 
 /* Threads compatibility routines for libgcc2 and libobjc.  */
 /* Compile this one with gcc.  */
-/* Copyright (C) 1997, 1999, 2000, 2001, 2004, 2005, 2008, 2009
-   Free Software Foundation, Inc.
+/* Copyright (C) 1997-2012 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -557,6 +556,12 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)
   return __gthread_mutex_unlock (__mutex);
 }
 
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+  return __gthread_mutex_destroy (__mutex);
+}
+
 #endif /* _LIBOBJC */
 
 #endif
diff --git a/libgcc/config/s390/gthr-tpf.h b/libgcc/config/s390/gthr-tpf.h
index fb23e91..49bce4c 100644
--- a/libgcc/config/s390/gthr-tpf.h
+++ b/libgcc/config/s390/gthr-tpf.h
@@ -1,6 +1,6 @@ 
 /* Threads compatibility routines for libgcc2 and libobjc.
    Compile this one with gcc.
-   Copyright (C) 2004, 2005, 2008, 2009 Free Software Foundation, Inc.
+   Copyright (C) 2004-2012 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -225,5 +225,10 @@  __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex)
   return 0;
 }
 
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+  return __gthread_mutex_destroy (__mutex);
+}
 
 #endif /* ! GCC_GTHR_TPF_H */
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h
index cc4e518..1e7ddfe 100644
--- a/libgcc/gthr-posix.h
+++ b/libgcc/gthr-posix.h
@@ -832,6 +832,12 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)
   return __gthread_mutex_unlock (__mutex);
 }
 
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+  return __gthread_mutex_destroy (__mutex);
+}
+
 #ifdef _GTHREAD_USE_COND_INIT_FUNC
 static inline void
 __gthread_cond_init_function (__gthread_cond_t *__cond)
diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h
index 4e39679..717e6cb 100644
--- a/libgcc/gthr-single.h
+++ b/libgcc/gthr-single.h
@@ -1,7 +1,6 @@ 
 /* Threads compatibility routines for libgcc2 and libobjc.  */
 /* Compile this one with gcc.  */
-/* Copyright (C) 1997, 1999, 2000, 2004, 2008, 2009
-   Free Software Foundation, Inc.
+/* Copyright (C) 1997-2012 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -286,6 +285,12 @@  __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)
   return __gthread_mutex_unlock (__mutex);
 }
 
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+  return __gthread_mutex_destroy (__mutex);
+}
+
 #endif /* _LIBOBJC */
 
 #undef UNUSED
diff --git a/libgcc/gthr.h b/libgcc/gthr.h
index 813abe1..9f2b53d 100644
--- a/libgcc/gthr.h
+++ b/libgcc/gthr.h
@@ -1,7 +1,6 @@ 
 /* Threads compatibility routines for libgcc2.  */
 /* Compile this one with gcc.  */
-/* Copyright (C) 1997, 1998, 2004, 2008, 2009, 2011
-   Free Software Foundation, Inc.
+/* Copyright (C) 1997-2012 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -73,6 +72,7 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
      int __gthread_setspecific (__gthread_key_t key, const void *ptr)
 
      int __gthread_mutex_destroy (__gthread_mutex_t *mutex);
+     int __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex);
 
      int __gthread_mutex_lock (__gthread_mutex_t *mutex);
      int __gthread_mutex_trylock (__gthread_mutex_t *mutex);
diff --git a/libstdc++-v3/include/ext/concurrence.h b/libstdc++-v3/include/ext/concurrence.h
index ad02839..68c679c 100644
--- a/libstdc++-v3/include/ext/concurrence.h
+++ b/libstdc++-v3/include/ext/concurrence.h
@@ -219,7 +219,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     ~__recursive_mutex()
     {
       if (__gthread_active_p())
-	_S_destroy(&_M_mutex);
+	__gthread_recursive_mutex_destroy(&_M_mutex);
     }
 #endif
 
@@ -247,43 +247,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     __gthread_recursive_mutex_t* gthread_recursive_mutex(void)
     { return &_M_mutex; }
-
-#if __GTHREADS && ! defined __GTHREAD_RECURSIVE_MUTEX_INIT
-    // FIXME: gthreads doesn't define __gthread_recursive_mutex_destroy
-    // so we need to obtain a __gthread_mutex_t to destroy
-  private:
-    template<typename _Mx, typename _Rm>
-      static void
-      _S_destroy_win32(_Mx* __mx, _Rm const* __rmx)
-      {
-        __mx->counter = __rmx->counter;
-        __mx->sema = __rmx->sema;
-        __gthread_mutex_destroy(__mx);
-      }
-
-    // matches a gthr-win32.h recursive mutex
-    template<typename _Rm>
-      static typename __enable_if<(bool)sizeof(&_Rm::sema), void>::__type
-      _S_destroy(_Rm* __mx)
-      {
-        __gthread_mutex_t __tmp;
-        _S_destroy_win32(&__tmp, __mx);
-      }
-
-    // matches a recursive mutex with a member 'actual'
-    template<typename _Rm>
-      static typename __enable_if<(bool)sizeof(&_Rm::actual), void>::__type
-      _S_destroy(_Rm* __mx)
-      { __gthread_mutex_destroy(&__mx->actual); }
-
-    // matches when there's only one mutex type
-    template<typename _Rm>
-      static typename
-      __enable_if<std::__are_same<_Rm, __gthread_mutex_t>::__value,
-        void>::__type
-      _S_destroy(_Rm* __mx)
-      { __gthread_mutex_destroy(__mx); }
-#endif
   };
 
   /// Scoped lock idiom.
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index 34d64c5..b28a53e 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -101,42 +101,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 
     ~__recursive_mutex_base()
-    { _S_destroy(&_M_mutex); }
-
-  private:
-    // FIXME: gthreads doesn't define __gthread_recursive_mutex_destroy
-    // so we need to obtain a __gthread_mutex_t to destroy
-
-    // matches when there's only one mutex type
-    template<typename _Rm>
-      static
-      typename enable_if<is_same<_Rm, __gthread_mutex_t>::value, void>::type
-      _S_destroy(_Rm* __mx)
-      { __gthread_mutex_destroy(__mx); }
-
-    // matches a recursive mutex with a member 'actual'
-    template<typename _Rm>
-      static typename enable_if<(bool)sizeof(&_Rm::actual), void>::type
-      _S_destroy(_Rm* __mx)
-      { __gthread_mutex_destroy(&__mx->actual); }
-
-    // matches a gthr-win32.h recursive mutex
-    template<typename _Rm>
-      static typename enable_if<(bool)sizeof(&_Rm::sema), void>::type
-      _S_destroy(_Rm* __mx)
-      {
-        __gthread_mutex_t __tmp;
-        _S_destroy_win32(&__tmp, __mx);
-      }
-
-    template<typename _Mx, typename _Rm>
-      static void
-      _S_destroy_win32(_Mx* __mx, _Rm const* __rmx)
-      {
-        __mx->counter = __rmx->counter;
-        __mx->sema = __rmx->sema;
-        __gthread_mutex_destroy(__mx);
-      }
+    { __gthread_recursive_mutex_destroy(&_M_mutex); }
 #endif
   };