Java multithreading problem about why this variable didn't change as hoped

Question

Sample code is as this:

package SynTest;

public class Test01 {

    public static void main(String[] args) {
        // TODO Auto-generated method stub
        Account account = new Account(100,"account");
        SafeDrawing a = new SafeDrawing(account,80,"a");
        SafeDrawing b = new SafeDrawing(account,80,"b");
        a.start();
        b.start();

    }

}

class Account{
    int money;
    String name;
    
    public Account(int money,String name) {
        this.money = money;
        this.name = name;
    }
}

class SafeDrawing extends Thread{
    volatile Account account;
    int drawingMoney;
    int packetTotal;
    
    public SafeDrawing(Account account,int drawingMoney,String name) {
        super(name);
        this.account = account;
        this.drawingMoney = drawingMoney;
    }
    
    public void test(){
        System.out.println(Thread.currentThread().getName()+" comes in！");
        if(account.money-drawingMoney<0) {
            return;
        }
        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        account.money -= drawingMoney;
        packetTotal += drawingMoney;
        System.out.println(this.getName()+"-->account remains："+account.money);
        System.out.println(this.getName()+"-->money in packet："+packetTotal);
        System.out.println(Thread.currentThread().getName()+" comes out！");
    }

    @Override
    public void run() {
        test();
    }
        
}

Two threads are startd in main, and each of them owns a same object named account. And each thread reduces the money property of account, which should be bigger than 0;

Now the method test() is obviously unsafe, so the account.money can be less than 0, just like:

b comes in！
a comes in！
b-->account remains：-60
a-->account remains：-60
a-->money in packet：80
b-->money in packet：80
a comes out！
b comes out！

But when I kept running this code once and once again, I found an output like this:

a comes in！
b comes in！
a-->account remains：20
b-->account remains：20
a-->money in packet：80
b-->money in packet：80
a comes out！
b comes out！

This is weird because since these two threads both have run to the line System.out.println(this.getName()+"-->account remains："+account.money);, the code account.money -= drawingMoney; must have been executed twice too, and why the remaining money is 20 rather than -60? Even if the happen-beefore is considered, since the account is defined as volatile, it's still impossible to be 20 rather than -60. I just cant't figure it out, and thanks for any idea.

score 1 · Accepted Answer · answered Aug 23 '20 at 19:31

1

To explain the output you saw, both threads saw the same value for account.money (100), subtracted the same value (80) from it, and wrote the same value back to it (20). There was no protection against concurrency issues, so both threads did exactly the same thing.

Declaring the account property of SafeDrawing does not make the money property of Account volatile. Even if it did, volatile does not work in this case.

This code:

account.money -= drawingMoney;

Would not be safe even if account.money was volatile. See this question and answer for details:

Is a volatile int in Java thread-safe?

answered Aug 23 '20 at 19:31

Simon G.

6,587
25
30

So if one thread has written back the to main memory, and another thread is ready to write back, even if it know the data is changed ,it will still write the data back from it's working space? – Yuki N Aug 23 '20 at 20:18
That is correct. All that volatile does is ensure that any read that happens after a write sees the most recently written value. It does not have "compare and set" semantics, nor does it lock a property whilst the thread uses it. – Simon G. Aug 23 '20 at 21:55

score -1 · Answer 2 · answered Aug 23 '20 at 18:33

-1

Try using AtomicInteger

The AtomicInteger class provides you with a int variable which can be read and written atomically, and which also contains advanced atomic operations like compareAndSet().

answered Aug 23 '20 at 18:33

Ganesh

54
3

The `volatile` qualifier just ensures every thread sees the most recently written value. That is not enough when two threads do simultaneous read-modify-write operations. They can both read the same most recently written value, both modify it, then write on top of each other. I strongly recommend *not* using `volatile` unless you are absolutely positive that the only possible race that you have is the one it solves. – David Schwartz Aug 23 '20 at 20:04
So if one thread has written back the to main memory, and another thread is ready to write back, even if it know the data is changed ,it will still write the data back from it's working space? – Yuki N Aug 23 '20 at 20:18
On X86 volatile doesn't force changes to be written to main memory. Caches are always consistent; it will never happen that one reads an older version of data from the cache after another core wrote a newer version of the data. Caches are the source of truth; memory might be out of sync. In theory it is possible that memory never gets updates. – pveentjer Aug 24 '20 at 07:27

Java multithreading problem about why this variable didn't change as hoped

2 Answers2