volatile is not a lock: 4 ISR bugs I have personally shipped to pump controllers

All four of these went to the field in products with my name in the git blame. All four passed bench testing. Sharing so you can ship different bugs.

Bug 1: the missing volatile

bool adc_done = false;               /* BUG: not volatile */

void ADC_IRQHandler(void) {
    adc_value = ADC1->DR;
    adc_done = true;
}

void read_adc(void) {
    start_conversion();
    while (!adc_done) { }            /* at -O2 this becomes while(1) */
    adc_done = false;
}

The compiler sees nothing in the loop changing adc_done, so it reads it once and loops forever. Debug build (-O0) works, release hangs. Fix: volatile bool adc_done. This one everybody knows. The next three are why I am writing this.

Bug 2: volatile, but read-modify-write

volatile uint32_t pulse_count;

void EXTI0_IRQHandler(void) { pulse_count++; }

void every_second(void) {
    flow = pulse_count * K;
    pulse_count = 0;                 /* BUG: a pulse can arrive between these two lines */
}

volatile makes every access real. It does not make two accesses one. If the interrupt fires between the read and the write of zero, that pulse is lost. Our flow meter under-read by ~0.3% at high flow. Customer noticed in the water bill, not us. Fix:

uint32_t primask = __get_PRIMASK();
__disable_irq();
uint32_t n = pulse_count;
pulse_count = 0;
__set_PRIMASK(primask);

Save and restore PRIMASK, don't blindly call __enable_irq(), because you might already be inside someone else's critical section and you just enabled interrupts for them.

Bug 3: the 49.7 day bug

volatile uint64_t ms_ticks;          /* incremented in SysTick */

uint64_t now = ms_ticks;             /* BUG on a 32-bit MCU */

On Cortex-M a 64-bit read is two 32-bit loads. If SysTick fires between them exactly when the low word rolls over from 0xFFFFFFFF to 0, you read the old high word with the new low word (or the reverse), and time jumps by 2^32 ms. That is 49.7 days. Our maintenance scheduler thought 49 days passed in one millisecond and raised "service overdue" on a brand new pump. Took months to see it twice. Fix, either disable interrupts around the read, or the read-twice trick:

uint64_t get_ticks(void) {
    uint32_t hi, lo;
    do {
        hi = ticks_hi;
        lo = ticks_lo;
    } while (hi != ticks_hi);        /* high word changed, read again */
    return ((uint64_t)hi << 32) | lo;
}

Bug 4: volatile does not order your other memory

uint8_t tx_buf[64];
volatile bool tx_ready;

fill(tx_buf);
tx_ready = true;                     /* DMA ISR starts sending when it sees this */

Volatile accesses are kept in order with each other. Normal accesses (the buffer) can be moved around them by the compiler. And on a Cortex-M7 with data cache (STM32H7), the buffer can still be sitting in cache when DMA reads RAM. You send old data. Fix: a compiler and hardware barrier, and on M7 clean the cache for that buffer before starting DMA:

fill(tx_buf);
SCB_CleanDCache_by_Addr((uint32_t *)tx_buf, sizeof tx_buf);
__DMB();
tx_ready = true;

(For the cache clean, the buffer should be 32-byte aligned and sized, or you clean your neighbour's data too.)

What volatile actually means

"Every access to this variable really happens, in order relative to other volatile accesses." That's all. Not atomic. Not a lock. Not a memory barrier for other variables. The Linux kernel people have a famous document on this: volatile considered harmful. Different world, same lesson.

Keep ISRs short, share as little as possible, and when you share, think "what if the interrupt fires between these two lines". It will. In field only.