stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Liviu Nicoara <nikko...@hates.ms>
Subject Re: STDCXX-1056 [was: Re: STDCXX forks]
Date Wed, 05 Sep 2012 12:47:17 GMT
On 09/05/12 00:51, Stefan Teleman wrote:
> On Tue, Sep 4, 2012 at 10:49 PM, Martin Sebor <msebor@gmail.com> wrote:
>
>>    template <class _CharT>
>>    inline string numpunct<_CharT>::grouping () const
>>    {
>>        if (!(_C_flags & _RW::__rw_gr)) {
>>
>>            numpunct* const __self = _RWSTD_CONST_CAST (numpunct*, this);
>>
>>           _RWSTD_MT_GUARD (_C_mutex);
>>
>>           // [try to] get the grouping first (may throw)
>>            // then set a flag to avoid future initializations
>>            __self->_C_grouping  = do_grouping ();
>>            __self->_C_flags    |= _RW::__rw_gr;
>>        }
>>
>>        return _C_grouping;
>>    }
>
> That's what I wanted to do originally - use a per-object mutex.

FWIW, it is pretty obvious to me _now_ that these assignments are not MT-safe, by themselves
and also in the context of the test guarding them.

You're right, access must be synchronized on a per-facet object basis. Since the data that
needs to be protected on assignment is instance data, the mutex used in the guard must be
a (yet to be added) instance mutex. That would make it binary incompatible and would go in
... 4.3.x?


>
> This works:
>
> template <class _CharT>
> inline string numpunct<_CharT>::grouping () const
> {
>      if (!(_C_flags & _RW::__rw_gr)) {
>
>          numpunct* const __self = _RWSTD_CONST_CAST (numpunct*, this);
>
>          _RWSTD_MT_STATIC_GUARD (_Type);

It seems to me that a "static" guard is excessive. The only thing that needs guarding is the
actual assignment to facet instance members -- a mutex instance member would suffice.

>
> And even so, this is still not thread-safe:
>
> Two different threads [ T1 and T2 ], seeking two different locales
> [en_US.UTF-8 and ja_JP.UTF-8], enter std::numpunct<_CharT>::grouping()
> at the same time - because they are running on two different cores.
> They both test for
>
>      if (!(_C_flags & _RW::__rw_gr))
>
> and then -- assuming the expression above evaluates to true -- one of
> them wins the mutex [T1], and the other one [T2] blocks on the mutex.
>
> When T1 is done and exits the function, the grouping is set to
> en_US.UTF-8 and the mutex is released. Now T2 acquires the mutex, and
> proceeds to setting grouping to ja-JP.UTF-8. Woe on the caller running
> from T1 who now thinks he got en_US.UTF-8, but instead he gets
> ja_JP.UTF-8, which was duly set so by T2, but T1 had no clue about it
> (remember, the std::string grouping _charT buffer is shared by the
> caller from T1 and the caller from T2).

I am pretty sure that in your example, they would operate on two different instances of the
facet class. The "static" guard would synchronize their running alright but through two different
facet instances, copying data from two different places of the locale database. In this respect
a "static" guard is excessive.

I would defer it to Martin to validate a course of action but to me it looks like the only
problem is the concurrent assignment to facet instance member variables inside the facet member
functions. Which could be easily and nicely solved with a plain guard on a mutex ordinary
member variable.

Thanks!

Liviu

Mime
View raw message