SERVER-113363 introduced the DataWithLockFreeReads class, which provides lock-free reads for arbitrarily sized values using atomic operations.

There are a couple of problems with it, the first one is that the class introduces data-races. Even though the class intends to prevent torn-reads, the C++ standard considers data races undefined behavior, so we should avoid them making sure all accesses in the "critical section" are through atomic objects, even if they are relaxed accesses.

The second problem is that the class is not properly synchronized, it allows torn reads. I'm attaching a benchmark that reproduces them quite reliably.

--------------------------------------------------------------------------------------
Benchmark                            Time             CPU   Iterations UserCounters...
--------------------------------------------------------------------------------------
BM_LockFreeReads/threads:8       0.886 ns         7.09 ns    135309024 mismatches=18.619k
BM_LockFreeReads/threads:16      0.593 ns         7.54 ns     77537376 mismatches=33.805k
BM_LockFreeReads/threads:32      0.376 ns         7.62 ns     90673088 mismatches=73.966k

If we assume a single writer producing new values over and over, it executes the following instructions:

1. load generation (seq_cst)
2. write payload (non-atomic / relaxed)
3. cas generation (seq_cst)
4. load generation (seq_cst)
5. write payload (non-atomic / relaxed)
6. cas generation (seq_cst)
7. ...

Operation 5) synchronizes with 6) and can't be reordered past it, but it doesn't synchronize with 4) or 3) and can be reordered back.

On the reader side, we have:

1. load generation (acquire)
2. read payload (non-atomic / relaxed)
3. load generation (acquire)

Operation 2) synchronizes with 1), it won't be reordered past it, but it doesn't synchronize with 3).

This means it's possible for a reader to see generation number 0, but read (part of) a value written in generation 2.

I see two solutions:

1. Use sequentially consistent loads/stores everywhere, including the payload writes and the generation loads in the reader
2. Allow payload reads/writes to be relaxed atomics and use sequentially consistent atomic fences before writing the payload and after the reading it.

This script showcases that even strengthening a bit the generation loads in the reader doesn't fix the problem:

int main() {
  atomic_int gen = 0; 
  atomic_int c0 = 0;
  atomic_int c1 = 0;
  {{{ 
  {
    gen.load().readsvalue(0);
    c1.store(3, mo_relaxed);
    cas_strong_explicit(gen, 0, 1, mo_seq_cst, mo_seq_cst);
    gen.load().readsvalue(1);
    c0.store(5, mo_relaxed);
    cas_strong_explicit(gen, 1, 2, mo_seq_cst, mo_seq_cst);
  } ||| {
    gen.load().readsvalue(0);
    c0.load(mo_relaxed).readsvalue(5);
    gen.load().readsvalue(0);
  }
  }}};
  return 0; }

It can be run in CppMem ( documentation ).

Example execution with the problematic ordering: