Georg Baum wrote:
> Jean-Marc Lasgouttes wrote:
>
>> OK, could you know explain the same in terms of Language::babel(), which
>> is a const function? Between which objects is the representation shared?
>
> The fact that Language::babel() is const does not matter, since
> std::basic_string::_M_rep() lies: It is a const method but returns a non-
> const pointer to internal data. Therefore Language::babel_ can be (and is)
> modified each time a copy is made through the const accessor
> Language::babel(). During export a lot of these copies is being made, e.g.
> in output_latex.cpp.
>
> Creating the copy is a subset of the example I showed: If you don't look
> at the subsequent assignment, but compare only the difference after
> executing the first and second line, then it looks like this:
>
> std::string s1("foo");
> assert(s1._M_rep()->_M_refcount == 0);
>
> Now do the assignment:
>
> std::string s2(s1);
> assert(s1._M_rep()->_M_refcount == 1);
>
> Whether this is a race condition depends on whether increasing the ref
> counter is atomic or not. The wikipedia page mentions that it can be
> implemented in a thread-safe way without using locks by compare-and-swap
> (see http://en.wikipedia.org/wiki/Compare-and-swap), but the actual
> implementation is so complicated that I could not verify whether this is
> used at a quick glance. I only know that no locks are used. So my whole
> theory might be wrong actually, we'll need further investigation.
OK, I found out more:
1) I could not find any write access to string instances created as copy of
Language::babel_. They are all const, so my first example is actually not
used here (but might be used elsewhere, one area to test would be our VCS
and graphics converter code, both use multiple threads as well).
2) Increasing the ref counter is an atomic operation (see
std::basic_string::_Rep::_M_refcopy(), but this is not enough. The operation
which needs to be atomic is the test whether a ref count increase is neded
plus the increasing, but this is not the case, since the method _M_grab() is
implemented using several helpers:
_CharT* _M_grab(const _Alloc& __alloc1, const _Alloc& __alloc2)
{
return (!_M_is_leaked() && __alloc1 == __alloc2)
? _M_refcopy() : _M_clone(__alloc1);
}
Conclusion: With GNU libstdc++, creating a copy of a std::basic_string does
actually modify internal data of the original string in a non-atomic way,
and therefore the programmer is responsible to protect these operations in
multithreaded environments by some locking mechanism.
If anybody still doubts that this is true please tell (but look at the
helgrind log in trac first, around line 972 you have a demonstration of what
happens when a copy of Language::babel_ is being made). I am now 100% sure
and if needed I can try to give better explanations.
I am going to think more about possible solutions to this problem, but this
will take a few days. For now thanks to everybody who took part in the
discussion, without the right questions which were asked I would not have
understood the problem completely.
Georg
