Bug Hunt: The Broken Double-Checked Locking
Pre-Java-5, double-checked locking was famously broken because the JMM allowed the constructor's writes to be reordered with the publication of the reference. A reader could see a non-null reference whose object's fields were still uninitialized. The fix is volatile (Java) or a static-holder idiom, and similar gotchas apply in Go and Python.
The puzzle
A Java codebase has a Singleton class with the pre-Java-5 DCL pattern (no volatile). The lead engineer says: "It's been like that since 2003. Never caused a problem."
Should the code change?
The honest answer If the team upgraded the JVM in 2005+ and the singleton is rarely contested, the bug almost certainly never fired in the worst form. But the code is technically still broken on the JMM level, and migrating from x86 to ARM, upgrading the JVM, or hitting an unusual JIT path can fire it. Fix it now or be the person blamed when it fires.
What to look for in the broken code
In the Java tab, the suspicious line is instance = new Singleton(). It looks atomic. It isn't.
The JVM is allowed to reorder this into:
- Allocate memory for
Singleton. - Write the (still-uninitialized!) reference into
instance. - Run the constructor (which sets
config).
Step 2 happens BEFORE step 3. If another thread reads instance between steps 2 and 3, it sees:
- A non-null reference (so the outer
iffails fast, no lock acquired). - An object whose fields are still uninitialized.
getInstance().config.someField then NPEs.
Why volatile fixes it
The JMM 5+ rule: a volatile write establishes happens-before with any subsequent volatile read of the same field. Critically, the constructor's writes happen-before the volatile write. So no thread can observe a non-null instance whose constructor hasn't finished, the JMM forbids it.
The pattern of fixes across languages
| Language | Right fix |
|---|---|
| Java | volatile field + DCL, OR static holder idiom |
| Go | sync.Once.Do(f) |
| Python | Module-level construction, OR functools.cache, OR full Lock |
| C++ | std::call_once, OR Meyers Singleton (function-local static) |
Modern Java preference The static-holder idiom (a private static inner class with the singleton instance) is what most Java style guides recommend today. It's:
- Correct without volatile, relies on JVM class-init guarantees.
- Lazy, Holder class loads only when
get()is called. - Lock-free fast path, no synchronization in user code at all.
Use DCL only for a non-singleton lazy-init pattern that requires it (e.g., per-key memoization).
What this teaches beyond singletons
The same bug pattern recurs
- "Lazy initialization of a shared cache entry", broken without proper sync.
- "Publishing an immutable object via a non-final field", same JMM rule.
- "Configuration loaded at startup, read on every request", same publication pattern.
The skill: when reading code that lazily creates an object and stores it in a shared field, ask "what's the memory barrier from creation to publication?" If the answer is "the synchronized block", make sure the field write is inside the block AND the field is volatile (or use a holder). If the answer is "I'm not sure," the code is broken.
Implementations
"Double-checked locking" tries to avoid acquiring the lock on the fast path. The first if (instance == null) is unsynchronized. Looks fine, but what does another thread see between new Singleton() and the assignment to instance?
1 class Singleton {
2 private static Singleton instance; // ← spot the bug
3 private final Config config;
4 private Singleton() { this.config = loadConfig(); }
5
6 public static Singleton get() {
7 if (instance == null) { // unsynchronized check
8 synchronized (Singleton.class) {
9 if (instance == null) {
10 instance = new Singleton(); // ← reorderable!
11 }
12 }
13 }
14 return instance;
15 }
16 }
17
18 // Caller: Singleton.get().config.someField
19 // → may NPE because config is still null after a non-null readThe bug: instance = new Singleton() is not one operation. It's: (1) allocate memory, (2) write the field reference, (3) run the constructor. The JMM allows (2) and (3) to be reordered, another thread can read a non-null instance whose constructor hasn't finished. The reader sees uninitialized fields. Fix #1: declare instance as volatile. The volatile-write rule prevents reordering across the assignment. Fix #2: static-holder idiom, which uses Java's class-init guarantees. Fix #3: enum singleton, the cleanest, but stylistically polarizing.
1 // Fix #1, volatile DCL (CORRECT since Java 5)
2 class Singleton {
3 private static volatile Singleton instance;
4 public static Singleton get() {
5 Singleton local = instance; // single read
6 if (local == null) {
7 synchronized (Singleton.class) {
8 local = instance;
9 if (local == null) {
10 local = new Singleton();
11 instance = local;
12 }
13 }
14 }
15 return local;
16 }
17 }
18
19 // Fix #2, static holder (PREFERRED for most cases)
20 class Singleton {
21 private Singleton() { /* expensive init */ }
22 private static class Holder {
23 static final Singleton INSTANCE = new Singleton();
24 }
25 public static Singleton get() { return Holder.INSTANCE; }
26 }
27
28 // Fix #3, enum singleton (Effective Java item 3)
29 enum Singleton { INSTANCE; public void doWork() { /* ... */ } }Key points
- •DCL trick: 'check, lock, check again, construct', saves a lock on the fast path
- •Bug: without volatile, the constructor's writes can be reordered with the assignment to the field
- •A reader sees a non-null reference; reads object fields; sees garbage / nulls
- •Static-holder idiom (Java) is simpler and uses class-init guarantees instead
- •Go's sync.Once is the right answer; in Python, module-level construction works
Follow-up questions
▸Why was DCL famously broken in Java before 1.5?
▸Why is the static-holder idiom safer than DCL?
▸Does volatile have a performance cost?
▸What's wrong with Python's DCL even with the GIL?
▸Should I use sync.Once for everything in Go?
Gotchas
- !Java: forgetting volatile on the singleton field is the classic broken-DCL bug
- !Java: the static-holder idiom doesn't run init until get() is called, true laziness
- !Python: DCL without holding the lock for both check and set has a race on free-threaded CPython
- !Go: sync.Once.Do swallows panics, wrap critical init with a panic-aware version if needed
- !Reading instance.someField on the fast path skips ALL synchronization, must guarantee instance was safely published
- !Enum singletons in Java prevent reflection-based instantiation; regular ones don't
Spring's bean container relies on safe publication patterns. Hibernate's session factory uses static-holder. Go's stdlib (e.g. crypto/rand initialization) uses sync.Once. Every framework that lazily initializes shared state has solved this problem.