这个 Java 类是线程安全的吗？

Question

这并不是我的作业，而是来自一些大学分配给学生的任务。我对解决方案感兴趣。

这个任务是创建一个类（Calc），它保存一个整数。两个方法add和mul应该将其加上或乘以这个整数。

同时设置了两个线程。一个线程应该调用c.add(3)十次，另一个线程应该调用c.mul(3)十次（当然是在同一个Calc对象上）。

Calc类应该确保操作交替进行（添加、乘、添加、乘、添加、乘，等等）。

我没有做过很多与并发相关的问题 - 更少的是使用Java。我想出了以下实现方法：

class Calc{

    private int sum = 0;
    //Is volatile actually needed? Or is bool atomic by default? Or it's read operation, at least.
    private volatile bool b = true;

    public void add(int i){
        while(!b){}
        synchronized(this){
                sum += i;
            b = true;   
        }
    }

    public void mul(int i){
        while(b){}
        synchronized(this){
            sum *= i;
            b = false;  
        }
    }

}

我想知道我的做法是否正确。而且在 while(b) 部分肯定有更加优雅的方法。

PS：方法的签名不允许更改。除此之外没有其它限制。

- s3rius

---

你可以使用 AtomicBoolean 和 AtomicInteger 代替。 - CoolBeans

@AviramSegal - 具体来说：这是一个线程连续调用mul()十次。而不是十个线程分别调用mul()。 - s3rius

无论是否线程安全，任何以这种方式使用布尔值作为学生作业的人都应该受到严惩。字段名b。太好了，真是太好了。 - Maarten Bodewes

@owlstead 嗯，我从来不会给布尔变量起这样的名字。但是对于这个示例代码来说，它非常简短，所以我觉得这并不是必要的。将布尔变量作为一种开关滥用是相当丑陋的，但是我真正关心的只是线程安全部分。 - s3rius

虽然我理解简洁的要求，但我看到很多新开发人员似乎采用了这种简洁的方式，而我必须使用他们编写的代码。但请原谅我离题了。重点是即使它是线程安全的，也不是正确的方法。 - Maarten Bodewes

Answer 1

尝试使用Lock接口：

class Calc {

    private int sum = 0;
    final Lock lock = new ReentrantLock();
    final Condition addition  = lock.newCondition(); 
    final Condition multiplication  = lock.newCondition(); 

    public void add(int i){

        lock.lock();
        try {
            if(sum != 0) {
                multiplication.await();
            }
            sum += i;
            addition.signal(); 

        } 
        finally {
           lock.unlock();
        }
    }

    public void mul(int i){
        lock.lock();
        try {
            addition.await();
            sum *= i;
            multiplication.signal(); 

        } finally {
           lock.unlock();
        }
    }
}

锁的工作原理类似于您的同步块。但是，如果另一个线程持有锁，则方法将在 .await() 处等待，直到调用 .signal() 为止。

- Jivings

---

如果您修改示例以包括两个条件（以处理操作的交替），我会点赞。 - Perception

嗯，我不确定使用相同的信号变量是否线程安全。可能存在这样一种情况：调用add的线程会发出allowAccess信号，然后重新获取锁并发现它已经被发出了信号。 - Tudor

你的意思是因为加法必须先执行吗？我忘记了这一点。 - Jivings

@Perception 编辑以使用两个条件。 - Jivings

有人尝试执行这段代码吗？这不会导致死锁吗？当线程T1获取锁并等待调用multiplication.await()时。当T2调用mul并获取锁并等待调用addition.await()时，就会导致死锁。 - Prashant Bhate

再次查看代码，我认为conditions甚至不需要。唯一的要求是sum先执行。在第一个线程释放其Lock之前，第二个线程不会执行Mul。 - Jivings

Answer 2

你所做的是一个繁忙循环：你正在运行一个仅在变量改变时才停止的循环。这种技术很差，因为它使CPU非常忙碌，而不是让线程等待标志位改变。

我会使用两个信号量：一个用于multiply，另一个用于add。在添加之前，add必须获取addSemaphore，完成后则释放一个许可给multiplySemaphore，反之亦然。

private Semaphore addSemaphore = new Semaphore(1);
private Semaphore multiplySemaphore = new Semaphore(0);

public void add(int i) {
    try {
        addSemaphore.acquire();
        sum += i;
        multiplySemaphore.release();
    }
    catch (InterrupedException e) {
        Thread.currentThread().interrupt();
    }
}

public void mul(int i) {
    try {
        multiplySemaphore.acquire();
        sum *= i;
        addSemaphore.release();
    }
    catch (InterrupedException e) {
        Thread.currentThread().interrupt();
    }
}

- JB Nizet

---

我认为ReentrantLock版本更易理解一些。话虽如此，这个版本似乎在初始状态方面更清晰，并且可能更具适应性，所以你可以+1。 - Maarten Bodewes

Answer 3

正如其他人所说，您的解决方案中需要使用volatile。此外，您的解决方案是自旋等待的，这可能会浪费相当多的CPU周期。尽管如此，就正确性而言，我没有发现任何问题。

我个人会使用一对信号量来实现此功能：

private final Semaphore semAdd = new Semaphore(1);
private final Semaphore semMul = new Semaphore(0);
private int sum = 0;

public void add(int i) throws InterruptedException {
    semAdd.acquire();
    sum += i;
    semMul.release();
}

public void mul(int i) throws InterruptedException {
    semMul.acquire();
    sum *= i;
    semAdd.release();
}

- NPE

---

我们恰好有同样的想法！ - JB Nizet

@JBNizet：是的，但你比我更快地找到了这个解决方案（在决定信号量是最佳解决方案之前，我花了很多时间尝试其他原语）。 - NPE

那么，开发并不是一个速度比赛。如果是的话，我肯定会输得很惨。不幸的是，在某些方面，stackoverflow 就是这样。 - Maarten Bodewes

Answer 4

是的，volatile 是必要的，并不是因为从一个 boolean 赋值给另一个变量时不具有原子性，而是为了防止该变量被缓存，从而使更新后的值不可见于读取它的其他线程。如果您关心最终结果，sum 也应该是 volatile。

话虽如此，使用 wait 和 notify 创建这种交错效果可能更加优雅。

class Calc{

    private int sum = 0;
    private Object event1 = new Object();
    private Object event2 = new Object();

    public void initiate() {
        synchronized(event1){
           event1.notify();
        }
    }

    public void add(int i){
        synchronized(event1) {
           event1.wait();
        }
        sum += i;
        synchronized(event2){
           event2.notify();
        }
    }

    public void mul(int i){
        synchronized(event2) {
           event2.wait();
        }
        sum *= i;
        synchronized(event1){
           event1.notify();
        }
    }
}

然后在启动两个线程后，调用initiate释放第一个线程。

- Tudor

---

当然，Java规范实际上并没有任何进展的保证。/ 我认为这里synchronized是不必要的。/ 但是忙等待呢？ - Tom Hawtin - tackline

+1 对于缓存问题的提醒。我以前读过相关内容，但是完全忘记了。 - s3rius

@s3rius：我已经进行了一些使用wait和notify的代码编辑。对我来说，这似乎更加清晰明了。 - Tudor

sum不必是volatile，因为它受同步块的保护（只要在这样的块内读取）。 - pron

对于你关于volatile的解释，我给予加一。我不确定之前的评论说没有它会被优化掉是否正确。你对此有什么看法吗？我理解它更多是关于可见性而非防止优化... - Toby

@Toby：像“synchronized”语句这样的同步点会引入隐式内存屏障。由于sum在两个屏障之间被更新，所以在这种情况下对volatile关键字没有太大用处。不过一般情况下最好还是保留它。 - Tudor

Answer 5

如果不使用volatile，优化器可能会将循环优化为if(b)while(true){}

但是您也可以使用wait和notify来实现相同的效果。

public void add(int i){

    synchronized(this){
        while(!b){try{wait();}catch(InterruptedException e){}}//swallowing is not recommended log or reset the flag
            sum += i;
        b = true;   
        notify();
    }
}

public void mul(int i){
    synchronized(this){
        while(b){try{wait();}catch(InterruptedException e){}}
        sum *= i;
        b = false;  
        notify();
    }
}

但在这种情况下（在同步块内检查b），不需要使用volatile关键字。

- ratchet freak

---

我会使用notifyAll，以防需求更改并且您可能有多个添加和/或乘法线程（在这种情况下，您将遇到死锁）。 - JB Nizet

你有没有任何参考资料来支持关于优化掉非易失性变量的说法？我理解这更多是关于可见性的问题...或者这是相同问题的一部分吗？任何帮助我更好地理解这个问题的链接都将不胜感激 :) - Toby

@Toby 请查看这篇博客文章。 - ratchet freak

Answer 6

嗯，你的解决方案存在一些问题。首先，volatile 不是为了保证原子性而是为了保证可见性。我不会在这里详细讲述，但你可以阅读有关 Java 内存模型 的更多信息。(是的，boolean 是原子的，但在这里无关紧要)。此外，如果你只在 synchronized 块内部访问变量，则它们不必是 volatile。

现在，我假设这是偶然的，但是你的 b 变量不仅在 synchronized 块内部访问，而且恰好是 volatile，因此实际上你的解决方案将起作用，但它既不符合惯用法也不被推荐，因为你正在等待 b 在忙循环内发生变化。你正在浪费 CPU 周期(这就是我们称之为自旋锁，并且有时可能很有用)。

一个符合惯用法的解决方案将如下所示：

class Code {
    private int sum = 0;
    private boolean nextAdd = true;

    public synchronized void add(int i) throws InterruptedException {
        while(!nextAdd )
            wait();
        sum += i;
        nextAdd = false;
        notify();
    }

    public synchronized void mul(int i) throws InterruptedException {
        while(nextAdd)
            wait();
        sum *= i;
        nextAdd = true;
        notify();
    }
}

- pron

---

问题 - while(!b) wait(); 和 if(!b) wait(); 之间是否有差别？ - s3rius

@s3rius 是的。您必须始终在while循环中测试监视器条件（等待()的条件）。这有两个原因。首先，您不知道线程为什么被唤醒（使用notify()或notifyAll()）。也许是因为您正在测试的条件已更改，也可能是其他原因。其次，在某些硬件/操作系统架构中，可能会出现虚假的线程唤醒，即没有任何好理由的唤醒。我相信这在大多数架构中都不会发生，但是，惯用法仍然是：您必须始终在while循环中测试等待的条件。 - pron

赞你对虚假唤醒和循环条件检查的描述，加一分 :) - Toby

Answer 7

该程序是完全线程安全的：

1. 布尔标志被设置为volatile，这样JVM就知道不要缓存值，并且保持对一个线程的写访问。

2. 两个关键部分锁定在当前对象上，这意味着一次只有一个线程可以访问。请注意，如果一个线程在同步块内部，其他线程不能进入任何其他关键部分。

以上适用于类的每个实例。例如，如果创建了两个实例，则线程可以同时进入多个关键部分，但每个实例每个关键部分只能限制一个线程。明白吗？

- Jaco Van Niekerk

---

但它可以被改进。这就是问题所在。 - Jivings