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

Reply via email to