同步的锁定对象错了吗？

Question

public class UnsafeCol implements Runnable{
    List<String> list = new ArrayList<>();
    @Override
    public  void run() {
        synchronized (list) {
            list.add(Thread.currentThread().getName());
        }

    }

    public static void main(String[] args) {
        UnsafeCol r = new UnsafeCol();
        for (int i = 0; i < 100; i++) {
            new Thread(r).start();
        }
        System.out.println(r.list.size());
    }
}

输出：84
描述：我总是得到不正确的大小，但是我使用了synchronized来锁定列表，为什么？

Answer 1

您正在启动所有线程...但您不会在打印大小之前等待它们全部完成。

为此，您需要为已启动的线程保留 List<Thread>，然后在每个线程上调用 join（在单独的循环中）。

我还建议保持 all 对列表的访问同步 - 包括最终打印输出的大小。 可能在这里是不必要的，但对于大多数线程代码，我倾向于认为“显然正确但可能效率低下”比“需要规范检查以验证正确性”要好。

所以你会有这样的东西：

import java.util.ArrayList;
import java.util.List;

class UnsafeCol implements Runnable {
    private final List<String> list = new ArrayList<>();
    @Override
    public void run() {
        synchronized (list) {
            list.add(Thread.currentThread().getName());
        }
    }

    public int size() {
        synchronized (list) {
            return list.size();
        }
    }
}

public class Test {
    public static void main(String[] args) throws InterruptedException {
        UnsafeCol col = new UnsafeCol();
        List<Thread> threads = new ArrayList<>();

        // Start 100 threads
        for (int i = 0; i < 100; i++) {
            Thread thread = new Thread(col);
            thread.start();
            threads.add(thread);
        }

        // Wait for all the threads to start
        for (Thread thread : threads) {
            thread.join();
        }

        // Print out the size
        System.out.println(col.size());
    }
}

（为了清楚起见，我将其分为两个类，以说明如何防止对列表的不同步访问。）

就我个人而言，我更喜欢在其唯一目的是同步的对象上进行同步，但这是一个稍微不同的问题。

Answer 2

这里有两个错误。首先，你不要等到线程完成他们的工作。其次，还需要同步对列表的访问：

public class UnsafeCol implements Runnable{
    List<String> list = new ArrayList<>();
    @Override
    public  void run() {
        synchronized (list) {
            list.add(Thread.currentThread().getName());
    }

}

public static void main(String[] args) {
    UnsafeCol r = new UnsafeCol();

        List<Thread> threads = new ArrayList<>();


        for (int i = 0; i < 100; i++) {
            Thread t = new Thread(r);
            t.start();
            threads.add(t);
        }

        for (Thread t: threads) {
            t.join();
        }

        synchronized(r.list) {
            System.out.println(r.list.size());
        }
    }
}