On 08/05/16 04:55, Martin Liška wrote:
Thank you for very intensive brainstorming ;) Well I still believe that
following code
is not thread safe, let's image following situation:
yeah, you're right.
we could do better by using compare_exchange storing value, and detect the race
I mentioned:
gcov_t expected, val;
atomic_load (&counter[0], &val, ...);
[thread 1]: value == 1, read val == 1 // scheduled here
gcov_t delta = val == value ? 1 : -1;
atomic_add (&counter[1], delta);
if (delta < 0) {
retry:
/* can we set counter[0]? */
atomic_load (&counter[1], &expected, ...);
if (expected < 0) {
bool stored = atomic_compare_exchange (&counter[0], &val, &value, ...);
if (!stored && val != value)
goto retry;
[thread 2]: value == 2, just updated counter[0] to 2
// after that [thread 1] continue, but wrongly does counter[1]++, but value !=
counter[0]
atomic_add (&counter[1], 2, ...);
Bah. but (a) does it matter enough? and (b) if so does changing the delta<0
handling to store a count of 1 solve it?: (answer: no)
gcov_t expected, val;
A:atomic_load (&counter[0], &val, ...);
gcov_t delta = val == value ? 1 : -1;
B:atomic_add (&counter[1], delta);
if (delta < 0) {
/* can we set counter[0]? */
C:atomic_load (&counter[1], &expected, ...);
if (expected < 0) {
D:atomic_store (&counter[0], &value);
E: atomic_store (&counter[1], 1);
}
atomic_add (&counter[1], 2, ...);
thread-1: value = 1, reads '1' at A
thread-2: value = 2, reads '1' at A
thread-2: decrements count @ B
thread-2: reads -1 at C
thread-2: write 2 at D
thread-2: stores 1 at E
thread-1: increments count @ B (finally)
So we still can go awry. But the code's simpler. Like you said, I don't think
it's possible to solve without an atomic update to both counter[0] & counter[1].
Well, I wrote attached test-case which should trigger a data-race, but TSAN is
silent:
I'm not too surprised. The race window is tiny and you put a printf in the
middle of one path. I suspect if you put a sleep/printf on the counter[1]
increment path you'll see it more often.
nathan