On 9/17/12 5:42 PM, Liviu Nicoara wrote:
>[...]
> However, after more careful thought, I think there is a problem there even
> though we don't have an objective proof for it, yet.
>
> The writes are not atomic and they function just like DCII, being subject to
> both compiler reordering and out of order execution. E.g., it is assumed that
> the writes to the _C_impsize and _C_impdata members occur in the program order,
> which would make unguarded reads/tests of _C_impsize safe. This is what the
> facet initialization and access code does, conceptually:
>
> if (_C_impsize) {
> // already initialized, use data
> }
> else {
> // initialize
> mutex.lock ();
> _C_impdata = get_data_from_database ();
> _C_impsize = get_data_size ();
> mutex.unlock ();
> }
>
> The mutex lock and unlock operations introduce memory barriers but they are not
> guaranteeing the order in which the two writes occur. If the writes would occur
> in the exact program order, unguarded reads and tests of _C_impsize would be
> safe. Unfortunately, a thread may see the _C_impsize variable being updated
> before the _C_impdata.
Elaborating on the above -- facet.cpp:~250, the code is actually closer to this:
if (_C_impsize) {
// already initialized, use _C_impdata
}
else {
// initialize
mutex.lock ();
if (_C_impsize)
return _C_impdata;
void* const pv = ...;
size_t sz = ...;
_C_impdata = pv; // 1
_C_impsize = sz; // 2
mutex.unlock ();
}
(1) and (2) can be reordered. Also, when mapping the STDCXX locale database,
facet.cpp:288, there is another defect writing the two variables:
_C_impdata = __rw_get_facet_data (cat, _C_impsize, 0, codeset);
^^^^^^^^^^^
|||||||||||
Set early in __rw_get_facet_data ---------+++++++++++
The only way to order the above writes, preserve the fast checking for
initialization, and generally salvaging the current algorithm is a full memory
barrier in between the writes, e.g.:
if (_C_impsize) {
// already initialized, use data
}
else {
// ...
_C_impdata = pv;
full_membar ();
_C_impsize = sz;
// ...
}
Atomics are part of C++11, I wonder if anybody here has working experience with
the atomic API. A mutex locking will not do because the lock is only acquiring
and the write before the lock can shift down.
Of course I could be wrong. Any input is welcome.
Thanks,
Liviu