You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Farid Zaripov <Fa...@epam.com> on 2007/09/28 20:02:14 UTC
[PATCH] 22.locale.moneypunct.mt.cpp test
The __rw_get_moneypunct () function in punct.cpp is not thread safe
for the same
facets. If this function is invoked with the same pfacet parameter in
dirrerent threads
simultaneously, then some one thread successfully create __rw_setlocale
object
but other threads will blocked by global mutex in __rw_setlocale ctor.
Then when
the mutex will be released after the updating pfacet->_C_data(), the
next thread will
do the same initialization as the first thread.
ChangeLog:
* punct.cpp (__rw_get_moneypunct): After creating __rw_setlocale
object check
if the locale data is initialized by another thread to prevent
multiple initialization\
of the locale data in facet.
----------
Index: punct.cpp
===================================================================
--- punct.cpp (revision 580097)
+++ punct.cpp (working copy)
@@ -308,6 +308,10 @@
// set all categories -- need LC_NUMERIC and LC_CTYPE
const __rw_setlocale clocale (locname, _RWSTD_LC_ALL);
+ if (pfacet->_C_data ())
+ // call self recursively on already initialized in another
thread `impdata'
+ return __rw_get_moneypunct (pfacet, flags);
+
// get the lconv data
const lconv* const pconv = localeconv ();
if (!pconv)
----------
Farid.
Re: [PATCH] 22.locale.moneypunct.mt.cpp test
Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>
>
> Martin Sebor wrote:
>> I see what you're saying: the other thread will end up overwriting
>> the data written by the first thread. The newly data should be the
>> same, so I wonder if this just a benign race condition or if it's
>> more sinister than that?
>>
>
> It is a memory leak in the failure case. The block allocated by the
> first thread will be lost when the pointer is overwritten by the second
> thread.
Got it. Thanks for clarifying it for me!
Martin
RE: [PATCH] 22.locale.moneypunct.mt.cpp test
Posted by Travis Vitek <tv...@quovadx.com>.
Martin Sebor wrote:
>
>I see what you're saying: the other thread will end up overwriting
>the data written by the first thread. The newly data should be the
>same, so I wonder if this just a benign race condition or if it's
>more sinister than that?
>
It is a memory leak in the failure case. The block allocated by the
first thread will be lost when the pointer is overwritten by the second
thread.
Travis
Re: [PATCH] 22.locale.moneypunct.mt.cpp test
Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
> The __rw_get_moneypunct () function in punct.cpp is not thread safe
> for the same
> facets. If this function is invoked with the same pfacet parameter in
> dirrerent threads
> simultaneously, then some one thread successfully create __rw_setlocale
> object
> but other threads will blocked by global mutex in __rw_setlocale ctor.
> Then when
> the mutex will be released after the updating pfacet->_C_data(), the
> next thread will
> do the same initialization as the first thread.
I see what you're saying: the other thread will end up overwriting
the data written by the first thread. The newly data should be the
same, so I wonder if this just a benign race condition or if it's
more sinister than that?
Either way, I agree that it should be fixed. I took the liberty
to add more detail to your comment in the patch below to make
it even more clear what's going on (I had some difficulty
following the logic at first so I think it might be helpful to
others).
>
>
> ChangeLog:
Can you reference the Jira bug here please?
> * punct.cpp (__rw_get_moneypunct): After creating __rw_setlocale
> object check
> if the locale data is initialized by another thread to prevent
> multiple initialization\
> of the locale data in facet.
>
> ----------
> Index: punct.cpp
> ===================================================================
> --- punct.cpp (revision 580097)
> +++ punct.cpp (working copy)
> @@ -308,6 +308,10 @@
> // set all categories -- need LC_NUMERIC and LC_CTYPE
> const __rw_setlocale clocale (locname, _RWSTD_LC_ALL);
>
> + if (pfacet->_C_data ())
> + // call self recursively on already initialized in another
> thread `impdata'
> + return __rw_get_moneypunct (pfacet, flags);
> +
I suggest:
+ if (pfacet->_C_data ()) {
+ // check to see if another thread may have set _C_data()
+ // while we were waiting for the lock in __rw_setlocale
+ // ctor above and, if so, call self recursively on the
+ // already initialized `impdata'
+ return __rw_get_moneypunct (pfacet, flags);
+ }
Thanks
Martin
RE: [PATCH] 22.locale.moneypunct.mt.cpp test
Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Martin Sebor [mailto:sebor@roguewave.com]
> Sent: Saturday, September 29, 2007 1:49 AM
> To: stdcxx-dev@incubator.apache.org
> Subject: Re: [PATCH] 22.locale.moneypunct.mt.cpp test
>
> Travis Vitek wrote:
> > The same problem exists for __rw_get_numpunct.
>
> In that case we should probably make the same changes in
> both, referencing the respective issues in each ChangeLog, i.e.,
> (presumably) STDCXX-453 in the patch below and STDCXX-452 in
> the one for __rw_get_numpunct.
Commited thus: http://svn.apache.org/viewvc?rev=580959&view=rev
and http://svn.apache.org/viewvc?rev=580961&view=rev
Farid.
Re: [PATCH] 22.locale.moneypunct.mt.cpp test
Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
> The same problem exists for __rw_get_numpunct.
In that case we should probably make the same changes in both,
referencing the respective issues in each ChangeLog, i.e.,
(presumably) STDCXX-453 in the patch below and STDCXX-452 in
the one for __rw_get_numpunct.
Martin
>
> Farid Zaripov wrote:
>> The __rw_get_moneypunct () function in punct.cpp is not thread safe
>> for the same
>> facets. If this function is invoked with the same pfacet parameter in
>> dirrerent threads
>> simultaneously, then some one thread successfully create __rw_setlocale
>> object
>> but other threads will blocked by global mutex in __rw_setlocale ctor.
>> Then when
>> the mutex will be released after the updating pfacet->_C_data(), the
>> next thread will
>> do the same initialization as the first thread.
>>
>>
>> ChangeLog:
>> * punct.cpp (__rw_get_moneypunct): After creating __rw_setlocale
>> object check
>> if the locale data is initialized by another thread to prevent
>> multiple initialization\
>> of the locale data in facet.
>>
>> ----------
>> Index: punct.cpp
>> ===================================================================
>> --- punct.cpp (revision 580097)
>> +++ punct.cpp (working copy)
>> @@ -308,6 +308,10 @@
>> // set all categories -- need LC_NUMERIC and LC_CTYPE
>> const __rw_setlocale clocale (locname, _RWSTD_LC_ALL);
>>
>> + if (pfacet->_C_data ())
>> + // call self recursively on already initialized in another
>> thread `impdata'
>> + return __rw_get_moneypunct (pfacet, flags);
>> +
>> // get the lconv data
>> const lconv* const pconv = localeconv ();
>> if (!pconv)
>> ----------
>>
>> Farid.
>>
RE: [PATCH] 22.locale.moneypunct.mt.cpp test
Posted by Travis Vitek <tv...@quovadx.com>.
The same problem exists for __rw_get_numpunct.
Farid Zaripov wrote:
>
> The __rw_get_moneypunct () function in punct.cpp is not thread safe
>for the same
>facets. If this function is invoked with the same pfacet parameter in
>dirrerent threads
>simultaneously, then some one thread successfully create __rw_setlocale
>object
>but other threads will blocked by global mutex in __rw_setlocale ctor.
>Then when
>the mutex will be released after the updating pfacet->_C_data(), the
>next thread will
>do the same initialization as the first thread.
>
>
> ChangeLog:
> * punct.cpp (__rw_get_moneypunct): After creating __rw_setlocale
>object check
> if the locale data is initialized by another thread to prevent
>multiple initialization\
> of the locale data in facet.
>
>----------
>Index: punct.cpp
>===================================================================
>--- punct.cpp (revision 580097)
>+++ punct.cpp (working copy)
>@@ -308,6 +308,10 @@
> // set all categories -- need LC_NUMERIC and LC_CTYPE
> const __rw_setlocale clocale (locname, _RWSTD_LC_ALL);
>
>+ if (pfacet->_C_data ())
>+ // call self recursively on already initialized in another
>thread `impdata'
>+ return __rw_get_moneypunct (pfacet, flags);
>+
> // get the lconv data
> const lconv* const pconv = localeconv ();
> if (!pconv)
>----------
>
>Farid.
>