This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
[Patch] Fix libstdc++/12352 (continued)
- From: Paolo Carlini <pcarlini at unitus dot it>
- To: Pétur Runólfsson <peturr02 at ru dot is>
- Cc: Gabriel Dos Reis <gdr at integrable-solutions dot net>, libstdc++ at gcc dot gnu dot org, Benjamin Kosnik <bkoz at redhat dot com>
- Date: Sun, 28 Sep 2003 19:21:41 +0200
- Subject: [Patch] Fix libstdc++/12352 (continued)
- References: <07D05A69A3D0C14FAEA60C3ACE8E5564028F55D0@mail.ru.is>
Pétur Runólfsson wrote:
There still appear to be memory leaks in the constructor
locale::_Impl::
_Impl(const char* __s, size_t __refs)
: _M_references(__refs), _M_facets_size(_GLIBCXX_NUM_FACETS)
At least, __cloc appears to be leaked if an exception is thrown,
and if exceptions are thrown from the lines that look like
_M_init_facet(new std::collate<char>(__cloc));
then all memory allocated up to that point is leaked.
This should be detectable with the testcase I attached to the PR
if the outer loop is run often enough.
Hi!
The below is supposed to address Pétur's concerns. Basically,
I wrapped all the _M_init_facet calls in the try block (in order
to do that I had to tweak slightly the order of the new vs the
assignments for _M_facets, _M_caches and _M_names, and also the
destructor, adding checks for null pointers) and also dealt with
__cloc. Besides running the testsuite, I tried Pétur's testcase
with command line values up to 200000.
Pétur, Gaby, do you agree that it's a further improvement?
Thanks,
Paolo.
PR libstdc++/12352 (cont)
* src/localename.cc (locale::_Impl::_Impl(const char*, size_t)):
Don't leak __cloc; don't leak if any of the _M_init_facet(...)
calls fail.
(locale::_Impl::_Impl(const _Impl&, size_t)): Tweak.
(locale::_Impl::~_Impl): Don't do anything if !_M_facets,
!_M_caches, !_M_names.
diff -prN libstdc++-v3-1/src/localename.cc libstdc++-v3/src/localename.cc
*** libstdc++-v3-1/src/localename.cc Fri Sep 26 02:22:00 2003
--- libstdc++-v3/src/localename.cc Sun Sep 28 18:49:52 2003
*************** namespace std
*** 91,108 ****
locale::_Impl::
~_Impl() throw()
{
! for (size_t __i = 0; __i < _M_facets_size; ++__i)
! if (_M_facets[__i])
! _M_facets[__i]->_M_remove_reference();
delete [] _M_facets;
! for (size_t __i = 0; __i < _M_facets_size; ++__i)
! if (_M_caches[__i])
! _M_caches[__i]->_M_remove_reference();
delete [] _M_caches;
! for (size_t __i = 0; __i < _S_categories_size; ++__i)
! delete [] _M_names[__i];
delete [] _M_names;
}
--- 91,111 ----
locale::_Impl::
~_Impl() throw()
{
! if (_M_facets)
! for (size_t __i = 0; __i < _M_facets_size; ++__i)
! if (_M_facets[__i])
! _M_facets[__i]->_M_remove_reference();
delete [] _M_facets;
! if (_M_caches)
! for (size_t __i = 0; __i < _M_facets_size; ++__i)
! if (_M_caches[__i])
! _M_caches[__i]->_M_remove_reference();
delete [] _M_caches;
! if (_M_names)
! for (size_t __i = 0; __i < _S_categories_size; ++__i)
! delete [] _M_names[__i];
delete [] _M_names;
}
*************** namespace std
*** 115,146 ****
_M_names = 0;
try
{
! _M_facets = new const facet*[_M_facets_size];
_M_caches = new const facet*[_M_facets_size];
_M_names = new char*[_S_categories_size];
! }
! catch(...)
! {
! delete [] _M_facets;
! delete [] _M_caches;
! __throw_exception_again;
! }
!
! for (size_t __i = 0; __i < _M_facets_size; ++__i)
! {
! _M_facets[__i] = __imp._M_facets[__i];
! _M_caches[__i] = __imp._M_caches[__i];
! if (_M_facets[__i])
! _M_facets[__i]->_M_add_reference();
! if (_M_caches[__i])
! _M_caches[__i]->_M_add_reference();
! }
! // Name all the categories.
! for (size_t __i = 0; __i < _S_categories_size; ++__i)
! _M_names[__i] = 0;
! try
! {
for (size_t __i = 0; __i < _S_categories_size; ++__i)
{
char* __new = new char[std::strlen(__imp._M_names[__i]) + 1];
--- 118,142 ----
_M_names = 0;
try
{
! _M_facets = new const facet*[_M_facets_size];
! for (size_t __i = 0; __i < _M_facets_size; ++__i)
! {
! _M_facets[__i] = __imp._M_facets[__i];
! if (_M_facets[__i])
! _M_facets[__i]->_M_add_reference();
! }
_M_caches = new const facet*[_M_facets_size];
+ for (size_t __i = 0; __i < _M_facets_size; ++__i)
+ {
+ _M_caches[__i] = __imp._M_caches[__i];
+ if (_M_caches[__i])
+ _M_caches[__i]->_M_add_reference();
+ }
_M_names = new char*[_S_categories_size];
! for (size_t __i = 0; __i < _S_categories_size; ++__i)
! _M_names[__i] = 0;
! // Name all the categories.
for (size_t __i = 0; __i < _S_categories_size; ++__i)
{
char* __new = new char[std::strlen(__imp._M_names[__i]) + 1];
*************** namespace std
*** 170,193 ****
try
{
_M_facets = new const facet*[_M_facets_size];
_M_caches = new const facet*[_M_facets_size];
_M_names = new char*[_S_categories_size];
! }
! catch(...)
! {
! delete [] _M_facets;
! delete [] _M_caches;
! __throw_exception_again;
! }
!
! for (size_t __i = 0; __i < _M_facets_size; ++__i)
! _M_facets[__i] = _M_caches[__i] = 0;
! // Name all the categories.
! for (size_t __i = 0; __i < _S_categories_size; ++__i)
! _M_names[__i] = 0;
! try
! {
const size_t __len = std::strlen(__s);
if (!std::strchr(__s, ';'))
{
--- 166,181 ----
try
{
_M_facets = new const facet*[_M_facets_size];
+ for (size_t __i = 0; __i < _M_facets_size; ++__i)
+ _M_facets[__i] = 0;
_M_caches = new const facet*[_M_facets_size];
+ for (size_t __i = 0; __i < _M_facets_size; ++__i)
+ _M_caches[__i] = 0;
_M_names = new char*[_S_categories_size];
! for (size_t __i = 0; __i < _S_categories_size; ++__i)
! _M_names[__i] = 0;
! // Name all the categories.
const size_t __len = std::strlen(__s);
if (!std::strchr(__s, ';'))
{
*************** namespace std
*** 212,258 ****
_M_names[__i] = __new;
}
}
}
catch(...)
{
this->~_Impl();
__throw_exception_again;
! }
!
! // Construct all standard facets and add them to _M_facets.
! _M_init_facet(new std::ctype<char>(__cloc, 0, false));
! _M_init_facet(new codecvt<char, char, mbstate_t>(__cloc));
! _M_init_facet(new numpunct<char>(__cloc));
! _M_init_facet(new num_get<char>);
! _M_init_facet(new num_put<char>);
! _M_init_facet(new std::collate<char>(__cloc));
! _M_init_facet(new moneypunct<char, false>(__cloc, __s));
! _M_init_facet(new moneypunct<char, true>(__cloc, __s));
! _M_init_facet(new money_get<char>);
! _M_init_facet(new money_put<char>);
! _M_init_facet(new __timepunct<char>(__cloc, __s));
! _M_init_facet(new time_get<char>);
! _M_init_facet(new time_put<char>);
! _M_init_facet(new std::messages<char>(__cloc, __s));
!
! #ifdef _GLIBCXX_USE_WCHAR_T
! _M_init_facet(new std::ctype<wchar_t>(__cloc));
! _M_init_facet(new codecvt<wchar_t, char, mbstate_t>(__cloc));
! _M_init_facet(new numpunct<wchar_t>(__cloc));
! _M_init_facet(new num_get<wchar_t>);
! _M_init_facet(new num_put<wchar_t>);
! _M_init_facet(new std::collate<wchar_t>(__cloc));
! _M_init_facet(new moneypunct<wchar_t, false>(__cloc, __s));
! _M_init_facet(new moneypunct<wchar_t, true>(__cloc, __s));
! _M_init_facet(new money_get<wchar_t>);
! _M_init_facet(new money_put<wchar_t>);
! _M_init_facet(new __timepunct<wchar_t>(__cloc, __s));
! _M_init_facet(new time_get<wchar_t>);
! _M_init_facet(new time_put<wchar_t>);
! _M_init_facet(new std::messages<wchar_t>(__cloc, __s));
! #endif
!
! locale::facet::_S_destroy_c_locale(__cloc);
}
// Construct "C" _Impl.
--- 200,246 ----
_M_names[__i] = __new;
}
}
+
+ // Construct all standard facets and add them to _M_facets.
+ _M_init_facet(new std::ctype<char>(__cloc, 0, false));
+ _M_init_facet(new codecvt<char, char, mbstate_t>(__cloc));
+ _M_init_facet(new numpunct<char>(__cloc));
+ _M_init_facet(new num_get<char>);
+ _M_init_facet(new num_put<char>);
+ _M_init_facet(new std::collate<char>(__cloc));
+ _M_init_facet(new moneypunct<char, false>(__cloc, __s));
+ _M_init_facet(new moneypunct<char, true>(__cloc, __s));
+ _M_init_facet(new money_get<char>);
+ _M_init_facet(new money_put<char>);
+ _M_init_facet(new __timepunct<char>(__cloc, __s));
+ _M_init_facet(new time_get<char>);
+ _M_init_facet(new time_put<char>);
+ _M_init_facet(new std::messages<char>(__cloc, __s));
+
+ #ifdef _GLIBCXX_USE_WCHAR_T
+ _M_init_facet(new std::ctype<wchar_t>(__cloc));
+ _M_init_facet(new codecvt<wchar_t, char, mbstate_t>(__cloc));
+ _M_init_facet(new numpunct<wchar_t>(__cloc));
+ _M_init_facet(new num_get<wchar_t>);
+ _M_init_facet(new num_put<wchar_t>);
+ _M_init_facet(new std::collate<wchar_t>(__cloc));
+ _M_init_facet(new moneypunct<wchar_t, false>(__cloc, __s));
+ _M_init_facet(new moneypunct<wchar_t, true>(__cloc, __s));
+ _M_init_facet(new money_get<wchar_t>);
+ _M_init_facet(new money_put<wchar_t>);
+ _M_init_facet(new __timepunct<wchar_t>(__cloc, __s));
+ _M_init_facet(new time_get<wchar_t>);
+ _M_init_facet(new time_put<wchar_t>);
+ _M_init_facet(new std::messages<wchar_t>(__cloc, __s));
+ #endif
+ locale::facet::_S_destroy_c_locale(__cloc);
}
catch(...)
{
+ locale::facet::_S_destroy_c_locale(__cloc);
this->~_Impl();
__throw_exception_again;
! }
}
// Construct "C" _Impl.