Visibility bug: a non-volatile stop flag

What this error actually means

One thread wrote `running = false`. Another thread reads `running` in a loop and never observes the write. There is no exception, no error, and no way to tell from the source that anything is wrong — the write happened and the read is right there.

The Java memory model does not promise that a write by one thread becomes visible to another unless there is a *happens-before* relationship between them. A plain write to a plain field creates no such relationship. Without one, the compiler, the JIT, and the hardware are all free to keep the value where it is cheapest: a register, a store buffer, a core-local cache line. Nothing is obliged to make it globally visible, ever.

The specific mechanism that usually turns "eventually visible" into "never visible" is the JIT, not the CPU. When C2 compiles a loop whose condition reads a non-volatile field that the loop body never modifies, it can treat that read as loop-invariant and hoist it out — loading the value once before the loop and then testing a register. That transformation is legal precisely because the absence of a happens-before edge means the compiler is entitled to assume no other thread is changing it. The result is `while (true)` in the compiled code, and no later write by any thread can affect it.

This also explains the most misleading property of the bug: it works until it does not. Interpreted and tier-1 compiled code re-reads the field each iteration, so the flag appears to work during start-up and in short tests. Once the method is hot enough for C2 — typically after several thousand iterations — the optimised version takes over and the loop becomes uninterruptible. A load test that runs for ten seconds passes; the same code in production never stops.

Declaring the field `volatile` fixes it because a volatile write and a subsequent volatile read of the same field establish a happens-before edge. The compiler may no longer cache the value across the loop, and the hardware is instructed to make the write visible. That is the entire fix — and it is a correctness fix, not a hint.

How to diagnose it

grep -rn -E "(private|protected|public)? *(static)? *boolean +(running|stopped|shutdown|active)" src/main/java | grep -v volatile

jcmd <pid> Thread.print | grep -A 4 "ingest-worker"

java -Xint Worker      # terminates
java Worker            # hangs once C2 compiles the loop

java -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly \
     -XX:CompileCommand=print,com.example.ingest.Worker::run Worker | head -80

worker.start();
Thread.sleep(2_000);            // let C2 compile the loop
worker.stop();
assertTrue(worker.joinFor(Duration.ofSeconds(2)), "worker ignored stop flag");

The fix

Declare the flag `volatile`. A volatile write followed by a volatile read of the same field creates the happens-before edge the JMM requires, and it forbids the compiler from caching the value across loop iterations. For a single boolean flag with no other invariants attached, this is the complete and correct fix — not a mitigation.

Prefer interruption to a custom flag for stopping threads. `Thread.interrupt()` plus a loop condition of `!Thread.currentThread().isInterrupted()` uses the platform mechanism, which also unblocks interruptible blocking calls. A hand-rolled flag cannot wake a thread that is parked in `queue.take()`; interruption can. Use both together: the flag for a graceful drain, interruption for the forced stop.

Use `AtomicBoolean` when you need to *act once* on the transition, not merely observe it. `compareAndSet(false, true)` guarantees exactly one caller performs the shutdown work, which a volatile boolean does not — and it carries the same visibility guarantees. Do not reach for it purely for visibility, where volatile is cheaper and clearer.

If the flag is read under a lock, write it under the *same* lock. Publication through a monitor works only when both sides use it; mixing a synchronised write with a plain read is the most common way to get this wrong while believing it is handled.

Never read the flag once into a local outside the loop, and treat any composite condition with mixed volatility as broken. Make every field involved in the loop condition volatile, so the correctness of the loop does not depend on which field happens to force a re-read.

Do not rely on `Thread.sleep`, logging, or a `synchronized` block inside the loop to make it work. These often do make the symptom disappear — a method call can inhibit the hoist and a synchronised block inserts a barrier — but they provide no guarantee, and the next refactor that removes the log line reintroduces a bug nobody will connect to it. Fix the declaration.

Make long-lived worker threads daemon threads *in addition* to fixing the flag, so a future stop bug degrades into a lost background task at exit rather than a process that cannot terminate. This is a backstop for exit, never a substitute for a working stop path.

// The loop may be compiled to `while (true)`: no happens-before edge exists,
// so the read is loop-invariant as far as the compiler is concerned.
private boolean running = true;

public void run() {
    while (running) {          // hoisted: value read once, then a register test
        ingestBatch();
    }
}
public void stop() { running = false; }   // write may never become visible

// Also broken: published under a lock, read without one
synchronized void stop() { running = false; }
public void run() { while (running) ingestBatch(); }   // plain read, no edge

// Correct: volatile establishes the happens-before edge and forbids hoisting
private volatile boolean running = true;

public void run() {
    while (running && !Thread.currentThread().isInterrupted()) {
        ingestBatch();
    }
}
public void stop() { running = false; }

// Better for a stoppable worker: the flag drains, the interrupt unblocks
public void shutdown() {
    running = false;      // graceful: finish the current batch and exit
    thread.interrupt();   // forced: also unblocks queue.take() and sleep()
}

// Use AtomicBoolean when exactly one caller must perform the shutdown work
private final AtomicBoolean stopped = new AtomicBoolean();
public void shutdown() {
    if (stopped.compareAndSet(false, true)) closeResources();
}

How to stop it coming back

• Treat any field written by one thread and read by another without synchronisation as a defect; `volatile` or a lock, never neither
• Prefer interruption over bespoke flags for thread termination, since it also unblocks blocking calls
• Write termination tests that warm the loop for seconds before setting the flag, so the JIT-compiled version is the one under test
• Run concurrency suites on AArch64 as well as x86 — a strong hardware memory model masks a large fraction of visibility bugs
• Never read a loop-control field into a local before the loop, and keep loop conditions built only from volatile or lock-protected state
• Make long-lived worker threads daemon threads as a backstop so a stop bug cannot prevent process exit
• Alarm on pods being SIGKILLed after the termination grace period; an unstoppable loop is a common and easily missed cause

FAQ

Related

Other errors engineers hit next to this one

• ERROR: canceling statement due to lock timeout (ALTER TABLE)
• ERROR: canceling statement due to conflict with recovery
• psycopg2.InterfaceError: connection already closed
• RecursionError: maximum recursion depth exceeded
• MemoryError: unable to allocate array
• QueuePool limit of size 5 overflow 10 reached, connection timed out
• DetachedInstanceError: instance is not bound to a Session
• RuntimeError: Event loop is closed