Source code comment justifying thread sanitizer failure in walCheckpoint is wrong
In the function `int walCheckpoint(...)` in file `wal.c` at line 1820
there is the following comment
/* Thread-sanitizer reports that the following is an unsafe read,
** as some other thread may be in the process of updating the value
** of the aReadMark slot. The assumption here is that if that is
** happening, the other client may only be increasing the value,
** not decreasing it. So assuming either that either the "old" or
** "new" version of the value is read, and not some arbitrary value
** that would never be written by a real client, things are still
** safe. */
u32 y = pInfo->aReadMark[i];
> assuming either that either the "old" or "new" version of the value is read, and not some arbitrary value that would never be written by a real client
This assumption is false according to cpprefeference .
Specifically, what happens here is a data race which is undefined
I do not know whether the code is unsafe in practice. It is possible
that it is deemed safe because synchronization happens elsewhere or
because common compilers on common architectures do not exhibit
undefined behavior in this case. Either way, the comment lacks detail
because it does not agree with the standard.
Even if the code is safe in practice today, it should still avoid
undefined behavior in order to avoid future problems.