Patchwork [v3] libstdc++/46455 - fix mutex and condvar leaks

login
register
mail settings
Submitter Jonathan Wakely
Date Nov. 18, 2010, 2:42 a.m.
Message ID <AANLkTimPd1rjFnvv=8nYQKWi+7jeFE+st0tiypQ3MSMY@mail.gmail.com>
Download mbox | patch
Permalink /patch/72032/
State New
Headers show

Comments

Jonathan Wakely - Nov. 18, 2010, 2:42 a.m.
This change adds destructors to the mutex types in the library, it's
not trivial and could do with more testing on platforms where thread
model != posix.  I hacked the gthr headers to reproduce leaks under
valgrind and think I've tested all the code paths, but I don't have
anything suitable to add to the testsuite.

        PR libstdc++/46455
        * include/std/mutex: Define destructors for mutex types which use an
        init function.
        * include/ext/concurrence.h: Likewise.

Tested x86_64-linux, I plan to commit this to trunk in the next day or two.

Patch

Index: include/std/mutex
===================================================================
--- include/std/mutex	(revision 166575)
+++ include/std/mutex	(working copy)
@@ -75,6 +75,8 @@  _GLIBCXX_BEGIN_NAMESPACE(std)
       // XXX EAGAIN, ENOMEM, EPERM, EBUSY(may), EINVAL(may)
       __GTHREAD_MUTEX_INIT_FUNCTION(&_M_mutex);
     }
+
+    ~mutex() { __gthread_mutex_destroy(&_M_mutex); }
 #endif
 
     mutex(const mutex&) = delete;
@@ -109,6 +111,45 @@  _GLIBCXX_BEGIN_NAMESPACE(std)
     { return &_M_mutex; }
   };
 
+#ifndef __GTHREAD_RECURSIVE_MUTEX_INIT
+  // FIXME: gthreads doesn't define __gthread_recursive_mutex_destroy
+  // so we need to obtain a __gthread_mutex_t to destroy
+  class __destroy_recursive_mutex
+  {
+    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);
+      }
+
+  public:
+    // matches a gthr-win32.h recursive mutex
+    template<typename _Rm>
+      static typename enable_if<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<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<is_same<_Rm, __gthread_mutex_t>::value, void>::type
+      _S_destroy(_Rm* __mx)
+      { __gthread_mutex_destroy(__mx); }
+  };
+#endif
+
   /// recursive_mutex
   class recursive_mutex
   {
@@ -118,17 +159,19 @@  _GLIBCXX_BEGIN_NAMESPACE(std)
   public:
     typedef __native_type* 			native_handle_type;
 
+#ifdef __GTHREAD_RECURSIVE_MUTEX_INIT
+    recursive_mutex() : _M_mutex(__GTHREAD_RECURSIVE_MUTEX_INIT) { }
+#else
     recursive_mutex()
     {
       // XXX EAGAIN, ENOMEM, EPERM, EBUSY(may), EINVAL(may)
-#ifdef __GTHREAD_RECURSIVE_MUTEX_INIT
-      __native_type __tmp = __GTHREAD_RECURSIVE_MUTEX_INIT;
-      _M_mutex = __tmp;
-#else
       __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION(&_M_mutex);
-#endif
     }
 
+    ~recursive_mutex()
+    { __destroy_recursive_mutex::_S_destroy(&_M_mutex); }
+#endif
+
     recursive_mutex(const recursive_mutex&) = delete;
     recursive_mutex& operator=(const recursive_mutex&) = delete;
 
@@ -177,16 +220,17 @@  _GLIBCXX_BEGIN_NAMESPACE(std)
   public:
     typedef __native_type* 		  	native_handle_type;
 
-    timed_mutex()
-    {
 #ifdef __GTHREAD_MUTEX_INIT
-      __native_type __tmp = __GTHREAD_MUTEX_INIT;
-      _M_mutex = __tmp;
+    timed_mutex() : _M_mutex(__GTHREAD_MUTEX_INIT) { }
 #else
+    timed_mutex()
+    {
       __GTHREAD_MUTEX_INIT_FUNCTION(&_M_mutex);
-#endif
     }
 
+    ~timed_mutex() { __gthread_mutex_destroy(&_M_mutex); }
+#endif
+
     timed_mutex(const timed_mutex&) = delete;
     timed_mutex& operator=(const timed_mutex&) = delete;
 
@@ -281,17 +325,19 @@  _GLIBCXX_BEGIN_NAMESPACE(std)
   public:
     typedef __native_type* 			native_handle_type;
 
+#ifdef __GTHREAD_RECURSIVE_MUTEX_INIT
+    recursive_timed_mutex() : _M_mutex(__GTHREAD_RECURSIVE_MUTEX_INIT) { }
+#else
     recursive_timed_mutex()
     {
       // XXX EAGAIN, ENOMEM, EPERM, EBUSY(may), EINVAL(may)
-#ifdef __GTHREAD_RECURSIVE_MUTEX_INIT
-      __native_type __tmp = __GTHREAD_RECURSIVE_MUTEX_INIT;
-      _M_mutex = __tmp;
-#else
       __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION(&_M_mutex);
-#endif
     }
 
+    ~recursive_timed_mutex()
+    { __destroy_recursive_mutex::_S_destroy(&_M_mutex); }
+#endif
+
     recursive_timed_mutex(const recursive_timed_mutex&) = delete;
     recursive_timed_mutex& operator=(const recursive_timed_mutex&) = delete;
 
Index: include/ext/concurrence.h
===================================================================
--- include/ext/concurrence.h	(revision 166575)
+++ include/ext/concurrence.h	(working copy)
@@ -36,6 +36,8 @@ 
 #include <exception>
 #include <bits/gthr.h> 
 #include <bits/functexcept.h>
+#include <bits/cpp_type_traits.h>
+#include <ext/type_traits.h>
 
 _GLIBCXX_BEGIN_NAMESPACE(__gnu_cxx)
 
@@ -161,6 +163,14 @@  _GLIBCXX_BEGIN_NAMESPACE(__gnu_cxx)
 #endif 
     }
 
+#if __GTHREADS && ! defined __GTHREAD_MUTEX_INIT
+    ~__mutex() 
+    { 
+      if (__gthread_active_p())
+	__gthread_mutex_destroy(&_M_mutex); 
+    }
+#endif 
+
     void lock()
     {
 #if __GTHREADS
@@ -211,6 +221,14 @@  _GLIBCXX_BEGIN_NAMESPACE(__gnu_cxx)
 #endif 
     }
 
+#if __GTHREADS && ! defined __GTHREAD_RECURSIVE_MUTEX_INIT
+    ~__recursive_mutex()
+    {
+      if (__gthread_active_p())
+	_S_destroy(&_M_mutex);
+    }
+#endif
+
     void lock()
     { 
 #if __GTHREADS
@@ -234,7 +252,44 @@  _GLIBCXX_BEGIN_NAMESPACE(__gnu_cxx)
     }
 
     __gthread_recursive_mutex_t* gthread_recursive_mutex(void)
-      { return &_M_mutex; }
+    { 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<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<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.
@@ -284,6 +339,14 @@  _GLIBCXX_BEGIN_NAMESPACE(__gnu_cxx)
 #endif 
     }
 
+#if __GTHREADS && ! defined __GTHREAD_COND_INIT
+    ~__cond() 
+    { 
+      if (__gthread_active_p())
+	__gthread_cond_destroy(&_M_cond); 
+    }
+#endif 
+
     void broadcast()
     {
 #if __GTHREADS