analysis of memory explosion; point of introduction outed

[Loren James Rittle <rittle at latour dot rsch dot comm dot mot dot com>]

Benjamin, sorry I didn't spot this issue when you first posted the
code and bits/concurrency.h.  It looked too pretty to be buggy.  Now
looking at <bits/libc-lock.h> on a random RH box, I suspect that the
exact idiom used there (I've not looked at any actual usage context in
glibc) must put the equivalent of __glibcxx_mutex_define_initialized
outside any function and/or CLASS, which you stripped from the macro,
must have equivalent storage class and scope as the protected object(s)...

While this (in src/local_init.cc):

  locale::locale() throw()
  { 
    _S_initialize(); 
    __glibcxx_mutex_define_initialized(lock);
    __glibcxx_mutex_lock(lock);
    _S_global->_M_add_reference();
    _M_impl = _S_global;
    __glibcxx_mutex_unlock(lock);
  }

[Another function attempting to protect access to _S_global not shown.]

looks correct at a glance; in practice, it has a major bug which I
suppose is only readily seen on platforms which allocate additional
memory for a mutex beyond the pthread_mutex_t (FreeBSD4 does, perhaps
not in complete compliance with POSIX).  Each and every constructor
call gets a new stack variable called lock.  Thus, concurrent
constructor calls (i.e. in different threads) will have a race on
_S_global (...if they ever indeed had a race on that variable...).

This is also the root cause of the memory explosion I reported last
week.  The gthr abstraction layer doesn't support (at least, very
well) non-bounded, dynamic allocation of mutex since there is no
support for ever calling pthread_mutex_destroy().  If one studied
(e.g.) rope, one might find a similar memory leak in some use cases
(that finally explains that ;-), however one would note that the
memory for the mutex is defined outside of any one function.  Perhaps
the gthr abstraction layer could be extended to address this issue but
that doesn't fix the root bug found above.

IMHO, there is absolutely no way to bind/hide the allocation of memory
for a __gthread_mutex_t inside any one function unless the protected
memory is also bound by that scope.  It appears to me that _S_global
is the protected memory in locale::locale() and locale::global() thus
allocation of the protecting mutex will have to be hoisted.  Or,
standing by my original response to the PR that caused its
introduction, removed entirely as "fixing" a non-bug...  Either way,
the PR submitted did not include a test case (not even one that
non-deterministically hit)...  If it had, then I'm now positive that
the related patch would not have fixed it...

I propose unrolling the patch until said test case is forthcoming.  If
people want, we could document more explicitly that we don't claim to
support dlopen C++ shared images with non-self-contained constructor
calls in the presence of threading.  Frankly, until I see a test case,
I refuse to believe there was even a bug there when people constructed
their shared images properly (at least when ELF dependencies are used
properly and the initial image explicitly depends on libstdc++.so).

Anyways, this is too tedious to continue thinking about without a
concrete failure in a test case.

Regards,
Loren