On 1 October 2012 20:22, Ian Lance Taylor wrote:
> On Sun, Sep 30, 2012 at 11:41 AM, Jonathan Wakely <[email protected]>
> 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.
Oops, yes.
> 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.
Good point ... I have a vague recollection that I looked into it and
decided they were the same, but I wrote this patch back in July and
don't remember the details. If anyone knows better please let me know.
The attached revised patch adds 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.
Done. I've also made __gthread_recursive_mutex_destroy return int,
unlike the Win32 __ghtread_mutex_destroy (see PR 53888 for that)
>> --- 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.
Nope, you're right.
I've replaced it with this, which is all __ghtread_mutex_destroy does anyway:
static inline int
__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t
* UNUSED(__mutex))
{
return 0;
}
Is that indentation right? (the asterisk is in the same column as the
parameter type in a fixed-width font.)
Thanks for the careful review.
The ChangeLog entry is the same, I haven't rested this because the
changes since the first patch are only to targets I don't have access
to.
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.
commit 7877201b61694279811c0276c92d85aaec2b30a2
Author: Jonathan Wakely <[email protected]>
Date: Tue Oct 2 01:40:46 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.
diff --git a/libgcc/config/gthr-rtems.h b/libgcc/config/gthr-rtems.h
index c5bd522..3bfa67b 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( [email protected] ) */
/* 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,14 @@ __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)
+{
+ /* This requires that recursive and non-recursive mutexes have the same
+ representation. */
+ 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 <[email protected]>.
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.c b/libgcc/config/i386/gthr-win32.c
index ab1b69f..fcb15df 100644
--- a/libgcc/config/i386/gthr-win32.c
+++ b/libgcc/config/i386/gthr-win32.c
@@ -1,8 +1,7 @@
/* Implementation of W32-specific threads compatibility routines for
libgcc2. */
-/* Copyright (C) 1999, 2000, 2002, 2004, 2008, 2009, 2011
- Free Software Foundation, Inc.
+/* Copyright (C) 1999-2012 Free Software Foundation, Inc.
Contributed by Mumit Khan <[email protected]>.
Modified and moved to separate file by Danny Smith
<[email protected]>.
@@ -259,3 +258,10 @@ __gthr_win32_recursive_mutex_unlock
(__gthread_recursive_mutex_t *mutex)
return 0;
}
+
+int
+__gthr_win32_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex)
+{
+ CloseHandle ((HANDLE) mutex->sema);
+ return 0;
+}
diff --git a/libgcc/config/i386/gthr-win32.h b/libgcc/config/i386/gthr-win32.h
index 53f8396..9656148 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 <[email protected]>.
This file is part of GCC.
@@ -430,6 +429,8 @@ extern int
__gthr_win32_recursive_mutex_trylock (__gthread_recursive_mutex_t *);
extern int __gthr_win32_recursive_mutex_unlock (__gthread_recursive_mutex_t *);
extern void __gthr_win32_mutex_destroy (__gthread_mutex_t *);
+extern int
+ __gthr_win32_recursive_mutex_destroy (__gthread_recursive_mutex_t *);
static inline int
__gthread_once (__gthread_once_t *__once, void (*__func) (void))
@@ -536,6 +537,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 __gthr_win32_recursive_mutex_destroy (__mutex);
+}
+
#else /* ! __GTHREAD_HIDE_WIN32API */
#include <windows.h>
@@ -761,6 +768,13 @@ __gthread_recursive_mutex_unlock
(__gthread_recursive_mutex_t *__mutex)
return 0;
}
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex)
+{
+ CloseHandle ((HANDLE) mutex->sema);
+ return 0;
+}
+
#endif /* __GTHREAD_HIDE_WIN32API */
#ifdef __cplusplus
diff --git a/libgcc/config/mips/gthr-mipssde.h
b/libgcc/config/mips/gthr-mipssde.h
index 34f9b6c..2ce2258 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 <[email protected]>
This file is part of GCC.
@@ -223,6 +223,13 @@ __gthread_recursive_mutex_unlock
(__gthread_recursive_mutex_t *__mutex)
return 0;
}
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t
+ * UNUSED(__mutex))
+{
+ return 0;
+}
+
#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
};
