Why did wrapping only the write in #pragma omp atomic still produce a race?

atomic only makes the single operation it's attached to indivisible — here, just the assignment max_val = v. The if (v > max_val) check that decided whether to even attempt that write happens completely outside the atomic's protection, so multiple threads can all see the same stale max_val, all decide they should write, and then race to write in an arbitrary order — the atomic guarantees the write itself won't corrupt memory, but says nothing about which write happens last.

Would reduction(max:max_val) have been a simpler fix here?

For this specific pattern — finding a maximum across independent loop iterations — yes, reduction(max:max_val) is both simpler and likely faster, using the same private-copy-then-combine strategy from the earlier reductions lesson. critical is the more general tool, useful when the compound operation isn't one of the handful reduction has a built-in operator for.

Is there a rule of thumb for choosing atomic vs critical vs lock?

Reach for atomic first for a single simple operation on a single variable (increment, add, a plain write) — it's the fastest when it applies. Reach for critical when multiple related statements need to execute as one unit, as this lesson's fix does. Reach for a lock (omp_lock_t) specifically when the synchronized region needs to span control flow that critical's block structure can't express, or when different critical sections need independent locks rather than sharing OpenMP's default one.

Atomics vs Locks, and the Race Hiding Inside a Check-Then-Act

Q: Why was #pragma omp atomic almost 5x faster than a lock for the same increment?

A lock (omp_set_lock/omp_unset_lock) is a general-purpose mutex with real overhead — it has to handle arbitrary critical sections of any size and contention behavior. An atomic operation maps directly to a hardware-supported instruction (like x86's LOCK-prefixed add) for one specific narrow operation, with none of a general lock's bookkeeping. The speed comes from atomics doing far less work, not from being a cleverer lock.

Q: How does wrapping the whole check-then-act in #pragma omp critical fix it?

critical makes the entire block — the comparison and the conditional write together — run as one indivisible unit per thread, so no other thread can read max_val in the middle of another thread's decision about whether to update it. That closes exactly the gap atomic left open: the check and the act now happen atomically as a pair, not just the act alone.

#pragma omp atomic is nearly 5x faster than a lock for a plain increment — but it only protects the single write it's attached to, not the if-check before it. Three widened-race runs give three different wrong answers to prove it.

By SkillByExample · Updated Sep 13, 2026

parallel-programming openmp atomics locks race-conditions

Two ways to protect a shared counter, one clear winner on speed — and a much subtler bug once the “protected” operation is more than a single write.

Lock vs atomic: measuring the same increment

// lock_counter.c
#include <stdio.h>
#include <omp.h>

int main() {
    long counter = 0;
    omp_lock_t lock;
    omp_init_lock(&lock);

    double start = omp_get_wtime();
    #pragma omp parallel for num_threads(8)
    for (int i = 0; i < 1000000; i++) {
        omp_set_lock(&lock);
        counter++;
        omp_unset_lock(&lock);
    }
    printf("lock-based counter = %ld, time = %.4f s\n", counter, omp_get_wtime() - start);
    omp_destroy_lock(&lock);
    return 0;
}

gcc -fopenmp -o lock_counter lock_counter.c
./lock_counter

lock-based counter = 1000000, time = 0.2077 s

// atomic_counter.c
#pragma omp parallel for num_threads(8)
for (int i = 0; i < 1000000; i++) {
    #pragma omp atomic
    counter++;
}

gcc -fopenmp -o atomic_counter atomic_counter.c
./atomic_counter

atomic counter = 1000000, time = 0.0424 s

Both correct — 0.0424s versus 0.2077s, a measured ~5x difference. atomic maps directly to a hardware-supported instruction for this one narrow operation; a lock is a general-purpose mutex built to handle arbitrary critical sections, and that generality costs real overhead a plain increment doesn’t need.

The bug: atomic protects the write, not the check before it

// broken_max.c
#include <stdio.h>
#include <omp.h>
#include <unistd.h>

int main() {
    long max_val = 0;
    int values[8] = {100, 900000, 200, 999999, 300, 400, 500, 600};
    #pragma omp parallel for num_threads(8) schedule(dynamic,1)
    for (int i = 0; i < 8; i++) {
        int v = values[i];
        if (v > max_val) {
            usleep(50000);  // widen the race window between check and write
            #pragma omp atomic write
            max_val = v;
        }
    }
    printf("max_val = %ld (expected 999999)\n", max_val);
    return 0;
}

gcc -fopenmp -o broken_max broken_max.c
./broken_max
./broken_max
./broken_max

max_val = 500 (expected 999999)
max_val = 400 (expected 999999)
max_val = 300 (expected 999999)

Three runs, three different wrong answers — never the actual maximum. The usleep widens a race that’s real even without it: every thread reads the same stale max_val (0) during its if check, before any of them have written yet, so all eight decide independently that their value is bigger and should be written. #pragma omp atomic only protects the assignment itself from corruption — it says nothing about which of the eight competing writes happens last, so whichever thread’s atomic write lands last wins, regardless of whether its value was actually the largest.

The fix: protect the whole check-then-act, not just the act

// fixed_max.c
#pragma omp parallel for num_threads(8) schedule(dynamic,1)
for (int i = 0; i < 8; i++) {
    int v = values[i];
    #pragma omp critical
    {
        if (v > max_val) {
            usleep(50000);
            max_val = v;
        }
    }
}

gcc -fopenmp -o fixed_max fixed_max.c
./fixed_max
./fixed_max
./fixed_max

max_val = 999999 (expected 999999)
max_val = 999999 (expected 999999)
max_val = 999999 (expected 999999)

Correct, every time. critical makes the entire block — comparison and conditional write together — run as one indivisible unit per thread, so no thread can read max_val in the middle of another thread’s decision about whether to update it. That closes the exact gap atomic left open: the check and the act are now atomic as a pair, not just the write alone.

Practice

1. Predict what reduction(max:max_val) would give for the same data, and confirm it.

long max_val = 0;
int values[8] = {100, 900000, 200, 999999, 300, 400, 500, 600};
#pragma omp parallel for reduction(max:max_val)
for (int i = 0; i < 8; i++) {
    if (values[i] > max_val) max_val = values[i];
}
printf("max_val = %ld\n", max_val);

max_val = 999999

Correct, and simpler than either the broken or fixed version above — reduction(max:...) uses the same private-copy-then-combine strategy as reduction(+:sum) from the earlier lesson, which sidesteps the whole check-then-act race by never sharing max_val across threads during the loop at all.

2. Confirm the race in broken_max.c disappears without the artificial usleep widening it.

./broken_max

max_val = 999999 (expected 999999)

Often correct without the delay — the race is still there, but the window for two threads’ check-then-write sequences to actually overlap is microseconds wide instead of 50 milliseconds, so it’s far less likely to manifest on a given run. A bug that “usually doesn’t happen” is still a bug; the usleep in the original example exists specifically to make an already-present race reliably observable, not to create one.

3. Time the critical-section fix against the reduction version to see the cost of generality.

// fixed_max_timed.c -- same as fixed_max.c, minus the usleep, plus timing
double start = omp_get_wtime();
#pragma omp parallel for num_threads(8) schedule(dynamic,1)
for (int i = 0; i < 8; i++) {
    int v = values[i];
    #pragma omp critical
    {
        if (v > max_val) max_val = v;
    }
}
printf("critical-based max: %ld, time = %.4f s\n", max_val, omp_get_wtime() - start);

gcc -fopenmp -o timed_critical_max fixed_max_timed.c
./timed_critical_max

critical-based max: 999999, time = 0.0003 s

At just 8 iterations, the timing difference between critical and reduction is too small to matter — the real cost of critical’s generality only shows up at the scale the lock-vs-atomic comparison used (a million iterations), not here. Scale matters when choosing between correctness-equivalent synchronization strategies, not just which one is theoretically lighter.

Next: MPI — a different parallelism model entirely, for when threads sharing memory isn’t the right fit.

Lock vs atomic: measuring the same increment

The bug: atomic protects the write, not the check before it

The fix: protect the whole check-then-act, not just the act

Practice

Frequently Asked Questions