"Interesting" behavior in std::locale

[Jonathan Wakely]

On 31/10/14 14:56 -0400, James Benze wrote:
>You're right about setting global, I accidentally omitted that in my
>test.  My bad.  Regardless of that, it's simply more that happens
>inside the if block, which only happens once anyway.

But it would need to be set atomically and read atomically, so it's
not enough just to add an assignment in the if block.

>I was thinking that you were right about a data race, but I'm curious
>as to why we don't have the same problem then with _S_global (in
>local_init.cc).  It's set in locale::_S_initialize_once() and doesn't
>seem to have any include guards around it.

The first time _S_global is set is in _S_initialize_once() which is
serialized with pthread_once.

The standard says the implementation is not required to avoid data
races in accesses to the global locale, so later calls that read/write
_S_global are not required to be thread-safe.

>This was actually partially why I was so confused with
>locale::classic()...it sets _S_classic in the more conventional
>singleton pattern, and then does this weird thing for setting
>c_locale.  The multiple placement new pattern wouldn't work for
>_S_classic obviously, since it seems to contain a reference count and
>"will" change at points, unless the c_locale object, but since the
>library developer did something different for locale::classic, I was
>wondering if there was a purpose.

I'm sure there is a purpose, but I don't know what it is :-)

>Regardless though, I would think that either _S_classic is also a
>source for a possible data-race, or neither of them are.

The only write to _S_classic is in _S_initialize_once() which is
serialized with pthread_once() and so the write happens before all
reads.

>**********
>
>I'm also pretty sure you could get around the data race side effects with
>
>if(!global)
>   global = new (&c_locale) locale(_S_classic);
>   return global;
>return *(locale*)(&c_locale);

No, that's still got a data race if one thread is testing !global
while the other thread is writing to it.

>Concurrent writes on global is moot since every write is the same
>value...the address of the c_locale array.

That's still a data race.

>A concurrent read and
>write could be problematic, but the read would at least pick up
>non-null, so we avoid the problem by not referencing it again, and
>simply using the address of the c_locale array.

That's still a data race though.

>Interleaved reads
>before global is set gives us the same multiple placement new issue we
>have now which is already a problem.

I agree the current code looks like it has a data race in the
placement new writing to c_locale (which writes to the _M_impl member
of the new locale object being constructed), but I'd rather leave it
alone than replace it with something else that is known to also have a
data race!

>I pretty much hate this though, and would much rather use some sort of
>atomic, were I actually implementing a change to the library. :-)

Agreed.