stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <tvi...@quovadx.com>
Subject RE: thoughts on STDCXX-452 -- std::numpunct not thread safe
Date Wed, 26 Sep 2007 18:38:47 GMT
 

Martin Sebor wrote:
>

[snip]

>The code is also unsafe in the sense that the do_decimal_point()
>function isn't guaranteed to be called at most once when multiple
>threads call decimal_point() at the same time but that in itself
>shouldn't cause any real problems since the virtual function is
>(or should be) thread safe. The real problem, I think, is in the
>unsafe assignment to _C_decimal_point.

It should (or had better) be thread safe. If it isn't we'll find it :)


>We need to find a binary
>compatible way to assign in a thread safe way the result of the
>do_xxx() functions to the member variables. We also need to OR
>the flags member in a thread safe way so as to avoid clearing
>(or setting) some of the other bits.
>
>The first could be done by _RWSTD_ATOMIC_EXCHANGE():
>
>   const string_type __dp = do_decimal_point ();
>
>   _RWSTD_ATOMIC_EXCHANGE (__self->_C_decimal_point,
>                           __dp, __self->_C_mutex);
>
>For the second we might need to add _RWSTD_ATOMIC_OR().

I think the easiest solution is to assign the cached value and modify
the bitmask under the same lock.

template <class _CharT>
inline _TYPENAME numpunct<_CharT>::char_type
numpunct<_CharT>::decimal_point () const
{
    if (!(_C_flags & _RW::__rw_dp)) {

        numpunct* const __self = _RWSTD_CONST_CAST (numpunct*, this);

        const char_type __tmp = do_decimal_point ();

        // call virtual outside lock because it may acquire the same
        // lock internally and we want to avoid a recursive acquire.
        _RWSTD_MT_GUARD (__self->_C_lock);

        // ithis conditional isn't strictly necessary and could be
        // omitted.
        if (!(_C_flags & _RW::__rw_dp)) {

            // [try to] get the decimal point first (may throw)
            // then set a flag to avoid future initializations
            __self->_C_decimal_point  = __tmp;
            __self->_C_flags         |= _RW::__rw_dp;

        }
    }

    return _C_decimal_point;
}

Intel Thread Checker continues to complain about the outer if accessing
_C_flags, but I'm pretty sure that this code is safe. The only problem I
can think of is that the compiler or runtime system does some
instruction reordering or something like that [it's a possibility]. I
worry that the optimizer will think that __tmp is actually not really
necessery and will optimize it out, choosing to write directly to
__self->_C_decimal_point outside the lock. 

Travis

>
>Thoughts?
>
>Martin
>

Mime
View raw message