Double-checked locking without volatile

What this error actually means

There is no exception for this bug and no failing assertion inside the synchronized block. The lock is correct: only one thread ever constructs the instance. What is broken is the *publication* of that instance to threads that read the field without the lock.

The mechanism is a reordering the Java memory model explicitly permits. Writing the object’s fields inside the constructor and writing the reference into the static field are two independent stores to a thread that has no happens-before edge to observe them in order. Both the compiler and the hardware are free to make the reference store visible first — and on top of that, the JIT may inline the constructor and allocate-then-publish-then-initialise, which is a legal transformation for a non-volatile field. A second thread taking the fast path then reads a non-null reference whose fields are still at their default values.

So the failure is not "two instances get created". It is one instance, observed in a state that never existed logically: non-null reference, null or zero fields. The `NullPointerException` fires in a getter on a fully-initialised-looking object, which is why the reported stack trace never points at the singleton code.

Declaring the field `volatile` fixes it because, since JSR-133 in Java 5, a volatile write cannot be reordered with earlier writes and a volatile read cannot be reordered with later reads: the write to the reference has a happens-before relationship with everything the constructor did. Before Java 5 the volatile fix did not work either, which is the origin of the widely repeated claim that double-checked locking is simply broken — it is broken without volatile, and correct with it, on Java 5 and later.

Note that `final` fields do not rescue you. Final-field freeze semantics guarantee that a thread which reads the reference *through a properly published path* sees the finals initialised — but they say nothing about a reference published by a racing non-volatile write, which is exactly the path in question.

How to diagnose it

grep -rn -B 2 -A 12 "synchronized" src/main/java/**/[A-Z]*.java | grep -E "private static [A-Z]| instance"

java -XX:-TieredCompilation -XX:CompileThreshold=1000 -XX:+PrintCompilation Repro 2>&1 | grep Settings

./gradlew jcstress -Pjcstress.mode=stress

The fix

If you keep double-checked locking, declare the field `volatile` and read it into a local exactly once per check. The volatile write on publication gives you the happens-before edge that orders the constructor’s writes before any reader’s observation of the reference, and the local avoids the second volatile read on the fast path.

Prefer the lazy-initialisation holder idiom when the value has no constructor arguments. A private static nested class whose static field holds the instance is initialised by the classloader on first access to that class, and JVM class initialisation is specified to be thread-safe with a happens-before edge for every subsequent reader. There is no lock in your code, no volatile to forget, and the fast path is a plain static field read — this is both the simplest and the fastest option.

For a per-key lazy value, use `ConcurrentHashMap.computeIfAbsent`, which performs the check-and-create atomically per key and publishes the value safely. This replaces hand-written double-checked locking over a map, which is where most of these bugs actually live.

Never publish `this` from a constructor. Move registration, thread starts, and callback wiring into a separate `start()` step invoked by whoever created the object, so no reference escapes before construction completes.

Make lazily created objects immutable with final fields where you can. An immutable object is far more forgiving of a marginal publication path, and it removes the follow-on class of bugs where the object is published correctly and then mutated unsafely.

Do not reach for `synchronized` on the whole accessor as the fix unless you have measured that it matters. It is correct and it is usually fine — uncontended biased-free monitor acquisition is cheap — but the holder idiom is correct *and* has no acquisition at all, so there is rarely a reason to choose the lock.

// Broken: the reference may be visible before the constructor's writes
private static Settings instance;
static Settings get() {
    if (instance == null) {                    // unsynchronised read
        synchronized (Settings.class) {
            if (instance == null) instance = new Settings();
        }
    }
    return instance;                           // may have null fields
}

// Correct: volatile gives the publication a happens-before edge
private static volatile Settings instance;
static Settings get() {
    Settings s = instance;                     // one volatile read
    if (s == null) {
        synchronized (Settings.class) {
            s = instance;
            if (s == null) instance = s = new Settings();
        }
    }
    return s;
}

// Better, when there are no constructor arguments: class init does the work
private static final class Holder {
    static final Settings INSTANCE = new Settings();
}
static Settings get() { return Holder.INSTANCE; }

// Per-key lazy values: atomic check-and-create, safely published
private final ConcurrentHashMap<String, Tenant> tenants = new ConcurrentHashMap<>();
Tenant tenant(String id) { return tenants.computeIfAbsent(id, Tenant::load); }

How to stop it coming back

• Treat any mutable static or instance reference that is read without a lock and written with one as a review blocker unless it is `volatile`
• Default to the holder idiom for singletons so there is no publication code to get wrong
• Ban `this` escaping a constructor; enforce it with a static-analysis rule rather than convention
• Run concurrency-sensitive suites on AArch64 as well as x86 — a strongly ordered CPU is not a test environment for ordering bugs
• Use JCStress for any hand-written publication protocol; "we ran it a million times" is not evidence under the JMM
• Make lazily initialised objects immutable so a single safe publication is the whole correctness argument

FAQ

Related

Other errors engineers hit next to this one

• QueuePool limit of size 5 overflow 10 reached, connection timed out
• DetachedInstanceError: instance is not bound to a Session
• RuntimeError: Event loop is closed
• Task was destroyed but it is pending!
• Executing <Handle ...> took 2.418 seconds (blocked event loop)
• SettingWithCopyWarning: A value is trying to be set on a copy of a slice
• celery.exceptions.WorkerLostError: Worker exited prematurely
• requests.exceptions.ReadTimeout: HTTPSConnectionPool read timed out