多线程更新AtomicInteger未达预期值，求代码问题排查

你的问题核心出在Counter类的单例实现不是线程安全的，导致多线程环境下可能创建多个Counter实例，每个实例都持有自己独立的Holder对象（以及里面的AtomicInteger n）。当10个线程同时调用Counter.getInstance()时，会有概率创建多个Counter实例，部分线程操作的是不同的AtomicInteger变量，最终累加的结果自然就不是预期的1000000了。

为什么会出现多个Counter实例？ #

看你的getInstance()方法：

public static Counter getInstance() {
    if (counter == null) {
        counter = new Counter();
    }
    return counter;
}

这是典型的懒汉式单例，但完全没做线程安全保护。当多个线程同时进入if (counter == null)判断时，都会认为counter还没初始化，进而各自创建新的Counter实例。比如线程A和线程B同时检查到counter为null，都会执行counter = new Counter();，这就导致内存中存在多个Counter对象，每个对象的holder.n都是独立的AtomicInteger。

举个例子：如果有3个线程各自创建了一个Counter实例，那么最终的结果就是这3个AtomicInteger的累加值，总和肯定小于1000000。

修复方案 #

我们需要把Counter的单例实现改成线程安全的，这里提供几种常见且可靠的方式：

方案1：给getInstance方法加同步锁 #

最直接的方式是给getInstance()方法加上synchronized关键字，确保同一时间只有一个线程能进入方法执行初始化：

public class Counter {
    private static Counter counter;
    Holder holder = new Holder();

    public static synchronized Counter getInstance() {
        if (counter == null) {
            counter = new Counter();
        }
        return counter;
    }

    class Holder {
        AtomicInteger n = new AtomicInteger(0);
    }
}

方案2：双重检查锁定（DCL）+ volatile #

如果担心synchronized方法的性能开销（虽然这个场景下影响极小），可以用双重检查锁定，同时给counter变量加上volatile修饰，禁止指令重排：

public class Counter {
    private static volatile Counter counter; // 加volatile禁止指令重排
    Holder holder = new Holder();

    public static Counter getInstance() {
        if (counter == null) {
            synchronized (Counter.class) {
                if (counter == null) { // 二次检查，避免后续线程重复初始化
                    counter = new Counter();
                }
            }
        }
        return counter;
    }

    class Holder {
        AtomicInteger n = new AtomicInteger(0);
    }
}

方案3：使用枚举实现单例（推荐） #

枚举是Java中最安全的单例实现方式，天然线程安全，还能防止反序列化破坏单例：

enum Counter {
    INSTANCE;

    private Holder holder = new Holder();

    public Holder getHolder() {
        return holder;
    }

    class Holder {
        AtomicInteger n = new AtomicInteger(0);
    }
}

对应的调用代码要改成Counter.INSTANCE.getHolder().n.incrementAndGet();

另外，你原来用Thread.sleep(10000)等待线程执行完成的方式不太可靠——如果线程实际执行时间超过10秒，输出结果还是不对。更好的方式是调用thread.join()等待所有线程执行完毕：

for (Thread thread : list) {
    thread.start();
}
// 替换sleep为join，确保所有线程执行完成再输出
for (Thread thread : list) {
    try {
        thread.join();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}
System.out.println(Counter.getInstance().holder.n);

验证修复效果 #

修复单例的线程安全问题后，所有线程都会操作同一个AtomicInteger变量，incrementAndGet()是原子操作，最终结果就会稳定为1000000了。

内容的提问来源于stack exchange，提问作者zonyang