[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added CC||arch.jslin at gmail dot com --- Comment #38 from Jonathan Wakely 2011-03-31 17:59:22 UTC --- *** Bug 48391 has been marked as a duplicate of this bug. ***
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED --- Comment #37 from Jonathan Wakely 2010-11-18 18:58:35 UTC --- fixed for 4.6.0 I think this is a bit risky for the 4.5 branch, and isn't a regression as we've never cleaned up those mutexes (the change in shared_ptr behaviour may be a regression, but seems to be caused by a change in MinGW's port - maybe they can apply the fix to their release if necessary)
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #36 from Jonathan Wakely 2010-11-18 18:56:34 UTC --- Author: redi Date: Thu Nov 18 18:56:29 2010 New Revision: 166917 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166917 Log: 2010-11-18 Jonathan Wakely PR libstdc++/46455 * include/std/mutex: Define destructors for mutex types which use an init function. * include/ext/concurrence.h: Likewise. Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/ext/concurrence.h trunk/libstdc++-v3/include/std/mutex
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #35 from Jonathan Wakely 2010-11-18 02:45:09 UTC --- patch posted to http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01875.html
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #34 from Jonathan Wakely 2010-11-18 00:29:20 UTC --- (In reply to comment #33) > If we need __is_same / __are_same, maybe we can just include > bits/cpp_type_traits.h aha, thanks for the pointer to are_same, I was going to move __gnu_debug::__is_same into ext/type_traits.h and use that, I didn't know about are_same. complete patch coming soon ...
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #33 from Paolo Carlini 2010-11-17 23:55:22 UTC --- If we need __is_same / __are_same, maybe we can just include bits/cpp_type_traits.h: isn't that big (and probably something else includes it anyway in most of the cases)
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #32 from Zouzou 2010-11-17 22:38:53 UTC --- (In reply to comment #31) > Created attachment 22436 [details] > add destructors in > a simpler version - could you test this on mingw? thanks great, compiles and fixes the resource leaks.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added Attachment #22427|0 |1 is obsolete|| --- Comment #31 from Jonathan Wakely 2010-11-17 22:13:11 UTC --- Created attachment 22436 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22436 add destructors in a simpler version - could you test this on mingw? thanks
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #30 from Jonathan Wakely 2010-11-17 09:37:37 UTC --- (In reply to comment #29) > because __gthread_mutex_destroy returns void. Argh, why is gthr-win32.h different here?! Nevermind ... > after changing the return types of the _S_destroy functions to void and > removing the "return" specifiers, the test case compiles and runs fine without > any leak. Great, thanks very much for testing. I'll get similar changes done to and get this checked in.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #29 from Zouzou 2010-11-17 07:38:39 UTC --- (In reply to comment #28) > Created attachment 22427 [details] > add destructors in > almost the same as the last patch but the destructor calls: > _S_destroy<__gthread_recursive_mutex_t>(&_M_mutex, 0); different error this time: In file included from c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/memory:78:0, from test.cpp:15: c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h: In static member function 'static int __gnu_cxx::__recursive_mutex::_S_destroy(_Rm*, typename __gnu_cxx::__recursive_mutex::_Detect_win32<_Rm, (& _Rm:: sema)>::type*) [with _Rm = __gthread_recursive_mutex_t, typename __gnu_cxx::__recursive_mutex::_Detect_win32<_Rm, (& _Rm:: sema)>::type = __gthread_mutex_t]': c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h:224:54: instantiated from here c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h:267:46: error: void value not ignored as it ought to be because __gthread_mutex_destroy returns void. after changing the return types of the _S_destroy functions to void and removing the "return" specifiers, the test case compiles and runs fine without any leak.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added Attachment #22424|0 |1 is obsolete|| --- Comment #28 from Jonathan Wakely 2010-11-16 23:33:44 UTC --- Created attachment 22427 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22427 add destructors in almost the same as the last patch but the destructor calls: _S_destroy<__gthread_recursive_mutex_t>(&_M_mutex, 0);
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #27 from Jonathan Wakely 2010-11-16 23:22:24 UTC --- I don't think that's right either ... I need to find a better way to test this
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #26 from Jonathan Wakely 2010-11-16 23:03:08 UTC --- Oops, that patch was broken - could you change the recursive mutex destructor so it calls _S_destroy(&_M_mutex, 0) instead of _S_destroy(&_M_mutex) As I can only test on POSIX systems I didn't notice I'd copied the code wrong from my prototype version which I used to simulate the Win32 case.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #25 from Jonathan Wakely 2010-11-16 22:36:16 UTC --- (In reply to comment #24) > apparently the 1st overload doesn't match so it falls back to the 3rd one. bah! ok, thanks, back to the drawing board...
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #24 from Zouzou 2010-11-16 22:29:35 UTC --- (In reply to comment #23) > Created attachment 22424 [details] > add destructors in > here's another patch, this one uses SFINAE to select an appropriate overload > based on the type of __gthread_recursive_mutex_t and if necessary extracts a > __gthread_mutex_t from it to pass to __gthread_mutex_destroy > this will be wrong if a recursive mutex type needs more cleanup than just > destroying its non-recursive mutex, though I don't think that's a problem for > any current systems supported by gthreads. > could be improved upon by disabling the third _S_destroy overload when > recursive_mutex_type is not the same as mutex_type, but it's ugly enough > already. > Zouzou, could you test this on mingw? thanks same compiler error as for the previous patch: In file included from c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/memory:78:0, from test.cpp:15: c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h: In static member function 'static int __gnu_cxx::__recursive_mutex::_S_destroy(_Rm*, ...) [with _Rm = __gthread_recursive_mutex_t]': c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h:224:22: instantiated from here c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h:284:44: error: cannot convert '__gthread_recursive_mutex_t*' to '__gthread_mutex_t*' for argument '1' to 'void __gthread_mutex_destroy(__gthread_mutex_t*)' (my line numbers are off by 2 compared to your patches.) apparently the 1st overload doesn't match so it falls back to the 3rd one.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added Attachment #22413|0 |1 is obsolete|| --- Comment #23 from Jonathan Wakely 2010-11-16 21:38:01 UTC --- Created attachment 22424 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22424 add destructors in here's another patch, this one uses SFINAE to select an appropriate overload based on the type of __gthread_recursive_mutex_t and if necessary extracts a __gthread_mutex_t from it to pass to __gthread_mutex_destroy this will be wrong if a recursive mutex type needs more cleanup than just destroying its non-recursive mutex, though I don't think that's a problem for any current systems supported by gthreads. could be improved upon by disabling the third _S_destroy overload when recursive_mutex_type is not the same as mutex_type, but it's ugly enough already. Zouzou, could you test this on mingw? thanks
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455
--- Comment #22 from Jonathan Wakely 2010-11-16
12:32:36 UTC ---
... and when using gthr-mipssde.h / gthr-posix95.h / gthr-solaris.h:
static inline int
__recursive_mutex_destroy(__gthread_recursive_mutex_t* __rmutex)
{
return ___gthread_mutex_destroy(__mutex->actual);
}
Adding the right preprocessor checks to the library will be messy, because
configure might have made gthr-default.h a symlink to one of the headers above
and we don't have a macro to test. The right place for the destroy function is
in the gthr headers where __gthread_recursive_mutex_t is defined.
Hmm, I wonder if we could use sfinae, there are three cases:
- recursive mutex is the same type as the non-recursive mutex, or
- it has an "actual" member, or
- it has a "sema" member.
I'll try that this evening.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #21 from Paolo Carlini 2010-11-16 11:52:20 UTC --- Argh, I see. I think we should keep the option open, anyway, together with a huge FIXME in the code, of course. I also think we should try to explain the problem to the people actually maintaining the gthr stuff: if we can convince those people that the change is localized and at the library-level we can only do very ugly things, maybe we can win...
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455
--- Comment #20 from Jonathan Wakely 2010-11-16
11:05:44 UTC ---
OK, that will be ugly though. The cast from __gthread_recursive_mutex_t* to
__gthread_mutex_t* is not correct, because the "sema" member (the actual Win32
handle) is at a different offset in the two mutex types.
We want something like:
static inline int
__recursive_mutex_destroy(__gthread_recursive_mutex_t* __rmutex)
{
#ifdef _WIN32
__gthread_mutex_t __tmp = { };
__tmp.counter = __rmutex->counter;
__tmp.sema = __rmutex->sema;
__ghtread_mutex_t* __mutex = &__tmp;
#else
__ghtread_mutex_t* __mutex = __rmutex;
#endif
return ___gthread_mutex_destroy(__mutex);
}
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #19 from Paolo Carlini 2010-11-16 10:25:40 UTC --- Jon, sometimes finding a reviewer for those gthr changes takes a bit of time... and we are already in Stage 3... Thus, I would recommend doing our best to figure out first a decent library-only fix.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added Target Milestone|--- |4.6.0 --- Comment #18 from Jonathan Wakely 2010-11-16 09:14:29 UTC --- Thanks for testing it. The cast makes me uncomfortable so I'll look into adding a __gthread_recursive_mutex_destroy function...
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #17 from Zouzou 2010-11-16 08:37:02 UTC --- (In reply to comment #16) > Created attachment 22413 [details] > add destructors in > could you try applying this patch to ext/concurrence.h and let me know if it > works on Windows? I'm testing on Linux, FreeBSD and OpenBSD but they all use > the pthreads model and I don't have a Windows system to test on. hi, the patch first produced a trivial compiler error: In file included from c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/memory:7 8:0, from test.cpp:15: c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h: In destruct or '__gnu_cxx::__recursive_mutex::~__recursive_mutex()': c:\mingw\bin\../lib/gcc/mingw32/4.5.0/include/c++/ext/concurrence.h:224:35: erro r: cannot convert '__gthread_recursive_mutex_t*' to '__gthread_mutex_t*' for arg ument '1' to 'void __gthread_mutex_destroy(__gthread_mutex_t*)' i worked around it by making the pointer cast explicit in the __recursive_mutex destructor: __gthread_mutex_destroy(reinterpret_cast<__gthread_mutex_t*>(&_M_mutex)); it then compiles fine and correctly fixes the issue at hand.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #16 from Jonathan Wakely 2010-11-16 00:48:52 UTC --- Created attachment 22413 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22413 add destructors in could you try applying this patch to ext/concurrence.h and let me know if it works on Windows? I'm testing on Linux, FreeBSD and OpenBSD but they all use the pthreads model and I don't have a Windows system to test on.
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #15 from Jonathan Wakely 2010-11-15 21:54:40 UTC --- Butenhof's book says you don't need to destroy a mutex/condvar that was statically initialized, so given the FreeBSD bug I will only define the destructor when the __GTHREAD_XXX_INIT macro is not available
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|unassigned at gcc dot |redi at gcc dot gnu.org |gnu.org | --- Comment #14 from Jonathan Wakely 2010-11-15 18:54:09 UTC --- yes, it shouldn't be hard to fix
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 --- Comment #13 from Paolo Carlini 2010-11-15 18:32:23 UTC --- Thanks a lot Jon for tracking this down! Do you think we can fix it in time for 4.6.0?
[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455 Jonathan Wakely changed: What|Removed |Added Summary|shared_ptr consuming too|resource leaks due to |many semaphores on Windows |missing destructors for ||mutexes and condvars --- Comment #12 from Jonathan Wakely 2010-11-15 18:12:19 UTC --- Missing destructors: __gnu_cxx::__mutex __gnu_cxx::__recursive_mutex __gnu_cxx::__cond std::mutex std::recursive_mutex std::timed_mutex std::recursive_timed_mutex I'm not sure why these didn't cause a problem previously, the destructors aren't there in 4.4 either
