[Bug libstdc++/46455] New: shared_ptr consuming too many semaphores on Windows

[Bug libstdc++/46455] New: shared_ptr consuming too many semaphores on Windows

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

           Summary: shared_ptr consuming too many semaphores on Windows
           Product: gcc
           Version: 4.5.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: internet@123gen.com


Created attachment 22383
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22383
test case

hello,

i have just hit a bug in the implementation of shared_ptr in GCC 4.5 on
Windows. the bug wasn't present in GCC 4.4.
the bug is also specific to libstdc++: when using boost or MSVC, there is no
problem.

basically, any creation of a shared_ptr increases the amount of semaphores used
by the process. this can be viewed by running the Task Manager and looking into
the "Handles" column. the semaphores are never released and the count therefore
keeps increasing without rest.

the following simple prog shows it:
int main() {
    int n=0;
    while (++n < 100) {
        //shared_ptr<string> x = make_shared<string>("test");
        shared_ptr<string> x(new string("test"));
        Sleep(100);
    }
    return 0;
}
(using either make_shared or the constructor doesn't make a difference.)

when running it, you will notice a steady increase of the amount of handles,
reaching more than 100 handles after 10 seconds.
compile the same prog with MSVC or GCC 4.4 and notice that the amount of
handles doesn't grow.
of course, for this simple example, it is inoffensive, but programs that make a
more important use of shared_ptr can quickly run out of available handles and
crash in unexpected ways.

for reference, the whole discussion is on
<https://bugs.launchpad.net/dcplusplus/+bug/654040>.

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-12 20:14:38 UTC ---
I don't think anything in shared_ptr changed between 4.4 and 4.5, though I
don't know what thread layer is used by MinGW.

I'll look into it

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-12 20:25:19 UTC ---
What's the value of __gnu_cxx::__default_lock_policy on Mingw?
Is it the same for GCC 4.4 and 4.5?

Between 4.4 and 4.5 there are some changes to the ghtr-win32.h file which
provides the gthreads abstraction layer, but there are no significant changes
to shared_ptr.  So for the moment I think it's either a change in the atomic
operations supported by mingw or a change in the threading layer, not
shared_ptr

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #3 from Zouzou <internet at 123gen dot com> 2010-11-12 20:56:14 UTC ---
(In reply to comment #2)
> What's the value of __gnu_cxx::__default_lock_policy on Mingw?
> Is it the same for GCC 4.4 and 4.5?

it is 1 on both.
(i got it using std::cout << __gnu_cxx::__default_lock_policy; hope that's
correct.)

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-12 23:04:46 UTC ---
yes, that's fine, thanks

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktietz at gcc dot gnu.org

--- Comment #5 from Kai Tietz <ktietz at gcc dot gnu.org> 2010-11-14 11:58:43 UTC ---
(In reply to comment #2)
> What's the value of __gnu_cxx::__default_lock_policy on Mingw?
> Is it the same for GCC 4.4 and 4.5?
> 
> Between 4.4 and 4.5 there are some changes to the ghtr-win32.h file which
> provides the gthreads abstraction layer, but there are no significant changes
> to shared_ptr.  So for the moment I think it's either a change in the atomic
> operations supported by mingw or a change in the threading layer, not
> shared_ptr

Well, I've tested it with mingw-w64 toolchains (64-bit and 32-bit) and I
couldn't reproduce this issue. Nevertheless it could be related here to some
modifications done by mingw.org on their toolchain. So threading model is here
something to look for.

Kai

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-14 17:50:21 UTC ---
if mingw.org modify GCC then this bug should be reported there, as I don't see
anything wrong in the libstdc++ code

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-15 12:26:24 UTC ---
Could you try this, which is a simplified version of the shared_ptr refcounting
code and should have the same behaviour:

#include <ext/atomicity.h>
#include <ext/concurrence.h>
#include <assert.h>

static int count0 = 0;

struct X
{
    X() : count1(1), count2(1) { ++count0; }

    ~X() { assert( count1==0 ); assert( count2==0 ); --count0; }

    void release()
    {
        if (__gnu_cxx::__exchange_and_add_dispatch(&count1, -1) == 1)
        {
            if (__gnu_cxx::__exchange_and_add_dispatch(&count2, -1) == 1)
                delete this;
        }
    }

    __gnu_cxx::__mutex m;
    _Atomic_word count1;
    _Atomic_word count2;
};

int main()
{
    for (int i=0; i<100; ++i)
    {
        X* x = new X;
        x->release();
    }
    assert( count0 == 0 );
}

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #8 from Zouzou <internet at 123gen dot com> 2010-11-15 13:38:39 UTC ---
(In reply to comment #5)
> (In reply to comment #2)
> > What's the value of __gnu_cxx::__default_lock_policy on Mingw?
> > Is it the same for GCC 4.4 and 4.5?
> > 
> > Between 4.4 and 4.5 there are some changes to the ghtr-win32.h file which
> > provides the gthreads abstraction layer, but there are no significant changes
> > to shared_ptr.  So for the moment I think it's either a change in the atomic
> > operations supported by mingw or a change in the threading layer, not
> > shared_ptr
> Well, I've tested it with mingw-w64 toolchains (64-bit and 32-bit) and I
> couldn't reproduce this issue. Nevertheless it could be related here to some
> modifications done by mingw.org on their toolchain. So threading model is here
> something to look for.
> Kai

i have just tried with the mingw64 currently in Cygwin (GCC 4.5.1) and it
indeed doesn't exhibit this problem.

i then tried to get the value of __gnu_cxx::__default_lock_policy on mingw64
and there, it is 2 (whereas it is 1 on vanilla mingw).

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #9 from Zouzou <internet at 123gen dot com> 2010-11-15 13:46:33 UTC ---
(In reply to comment #7)
> Could you try this, which is a simplified version of the shared_ptr refcounting
> code and should have the same behaviour:
> #include <ext/atomicity.h>
> #include <ext/concurrence.h>
> #include <assert.h>
> static int count0 = 0;
> struct X
> {
>     X() : count1(1), count2(1) { ++count0; }
>     ~X() { assert( count1==0 ); assert( count2==0 ); --count0; }
>     void release()
>     {
>         if (__gnu_cxx::__exchange_and_add_dispatch(&count1, -1) == 1)
>         {
>             if (__gnu_cxx::__exchange_and_add_dispatch(&count2, -1) == 1)
>                 delete this;
>         }
>     }
>     __gnu_cxx::__mutex m;
>     _Atomic_word count1;
>     _Atomic_word count2;
> };
> int main()
> {
>     for (int i=0; i<100; ++i)
>     {
>         X* x = new X;
>         x->release();
>     }
>     assert( count0 == 0 );
> }

[i added a Sleep(100) call after x->release(); for easier diagnostics.]

this code produces the same behavior as my test case (increasing number of
semaphores, over 100 after 10 seconds) on both vanilla MinGW and mingw64.

to summarize: my test case (standard shared_ptr creation) worked fine on
mingw64 (which has a __gnu_cxx::__default_lock_policy = 2) but bugged on
vanilla MinGW (which has a __gnu_cxx::__default_lock_policy = 1); but this one
produces the bug in both cases.

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.11.15 14:13:00
     Ever Confirmed|0                           |1

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-15 14:13:00 UTC ---
(In reply to comment #9)
> 
> this code produces the same behavior as my test case (increasing number of
> semaphores, over 100 after 10 seconds) on both vanilla MinGW and mingw64.
> 
> to summarize: my test case (standard shared_ptr creation) worked fine on
> mingw64 (which has a __gnu_cxx::__default_lock_policy = 2) but bugged on
> vanilla MinGW (which has a __gnu_cxx::__default_lock_policy = 1); but this one
> produces the bug in both cases.

That's because my simpler test ignores the default_lock_policy, it always uses
a mutex, which is what shared_ptr does when __default_lock_policy=1

I've just looked at __gnu_cxx::__mutex and it doesn't have a destructor, so the
problem is probably just that we leak the mutex. This should show the same bug:

#include <ext/concurrence.h>

int main()
{
    for (int i=0; i<100; ++i)
        __gnu_cxx::__mutex m;
}

Could you test that?

We should add a destructor to __mutex, which calls __gthread_mutex_destroy. 

N.B. this could cause a problem on FreeBSD
http://www.freebsd.org/cgi/query-pr.cgi?pr=150889
so we might want to only call __gthread_mutex_destroy when we have used
__GTHREAD_MUTEX_INIT_FUNCTION

[Bug libstdc++/46455] shared_ptr consuming too many semaphores on Windows

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #11 from Zouzou <internet at 123gen dot com> 2010-11-15 16:30:21 UTC ---
(In reply to comment #10)
> That's because my simpler test ignores the default_lock_policy, it always uses
> a mutex, which is what shared_ptr does when __default_lock_policy=1
> I've just looked at __gnu_cxx::__mutex and it doesn't have a destructor, so the
> problem is probably just that we leak the mutex. This should show the same bug:
> #include <ext/concurrence.h>
> int main()
> {
>     for (int i=0; i<100; ++i)
>         __gnu_cxx::__mutex m;
> }
> Could you test that?
> We should add a destructor to __mutex, which calls __gthread_mutex_destroy. 
> N.B. this could cause a problem on FreeBSD
> http://www.freebsd.org/cgi/query-pr.cgi?pr=150889
> so we might want to only call __gthread_mutex_destroy when we have used
> __GTHREAD_MUTEX_INIT_FUNCTION

this does reproduce the issue on both vanilla MinGW and MinGW64.

[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> 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 <redi at gcc dot gnu.org> 2010-11-15 18:12:19 UTC ---
Missing destructors:

<ext/concurrence.h>
__gnu_cxx::__mutex
__gnu_cxx::__recursive_mutex
__gnu_cxx::__cond

<mutex>
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

[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #13 from Paolo Carlini <paolo.carlini at oracle dot com> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> 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 <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-16 00:48:52 UTC ---
Created attachment 22413
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22413
add destructors in <ext/concurrence.h>

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

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #17 from Zouzou <internet at 123gen dot com> 2010-11-16 08:37:02 UTC ---
(In reply to comment #16)
> Created attachment 22413 [details]
> add destructors in <ext/concurrence.h>
> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.6.0

--- Comment #18 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #19 from Paolo Carlini <paolo.carlini at oracle dot com> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #20 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #21 from Paolo Carlini <paolo.carlini at oracle dot com> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #22 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22413|0                           |1
        is obsolete|                            |

--- Comment #23 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-16 21:38:01 UTC ---
Created attachment 22424
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22424
add destructors in <ext/concurrence.h>

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

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #24 from Zouzou <internet at 123gen dot com> 2010-11-16 22:29:35 UTC ---
(In reply to comment #23)
> Created attachment 22424 [details]
> add destructors in <ext/concurrence.h>
> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #25 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #26 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #27 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22424|0                           |1
        is obsolete|                            |

--- Comment #28 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-16 23:33:44 UTC ---
Created attachment 22427
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22427
add destructors in <ext/concurrence.h>

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

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #29 from Zouzou <internet at 123gen dot com> 2010-11-17 07:38:39 UTC ---
(In reply to comment #28)
> Created attachment 22427 [details]
> add destructors in <ext/concurrence.h>
> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #30 from Jonathan Wakely <redi at gcc dot gnu.org> 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 <mutex> and get this checked in.

[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22427|0                           |1
        is obsolete|                            |

--- Comment #31 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-11-17 22:13:11 UTC ---
Created attachment 22436
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22436
add destructors in <ext/concurrence.h>

a simpler version - could you test this on mingw?  thanks

[Bug libstdc++/46455] resource leaks due to missing destructors for mutexes and condvars

[internet at 123gen dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #32 from Zouzou <internet at 123gen dot com> 2010-11-17 22:38:53 UTC ---
(In reply to comment #31)
> Created attachment 22436 [details]
> add destructors in <ext/concurrence.h>
> 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

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #33 from Paolo Carlini <paolo.carlini at oracle dot com> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #34 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #35 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

--- Comment #36 from Jonathan Wakely <redi at gcc dot gnu.org> 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  <jwakely.gcc@gmail.com>

    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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #37 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46455

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |arch.jslin at gmail dot com

--- Comment #38 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-03-31 17:59:22 UTC ---
*** Bug 48391 has been marked as a duplicate of this bug. ***