这段代码为什么不是线程同步的？

Question

写了一个小程序，我觉应该是同步的，但结果不正确。程序是模拟银行账户的流水public class Account {        private int money = 60000;    public int getMoney() {        return money;    }        public void setMoney(int money) {        this.money = money;    }        }public class Save implements Runnable{        private Account account;        public Save(Account account) {        this.account=account;    }    @Override    public void run() {                    for(int i=0;i<50000;i++){                synchronized (account) {  //连续执行一定放入同步快中                    account.setMoney(account.getMoney()+1);//                    System.out.println("向账户存了1元，账户余额"+account.getMoney()+"元");                }            }            }}public class Take implements Runnable{        private Account account;        public Take(Account account) {        this.account=account;    }    @Override    public void run() {                    for(int i=0;i<50000;i++){                synchronized (account) {                    if(account.getMoney() > 0){                        account.setMoney(account.getMoney()-1);//                        System.out.println("向账户取了1元，账户余额"+account.getMoney()+"元");                    }else{//                        System.out.println("账户余额不足");                    }                }            }                    }}public class Main {    public static void main(String[] args) {        // TODO Auto-generated method stub        Account account = new Account();                Save save = new Save(account);        Take take = new Take(account);                Thread thread1 = new Thread(save);        Thread thread2 = new Thread(take);                System.out.println("账户余额为"+account.getMoney());        System.out.println("开始");                thread1.start();        thread2.start();        System.out.println("最后账户余额为："+account.getMoney());    }    }存钱和取钱的钱数是一样的，所以总数应该还是60000，但结果不是这个数。是不是我Main线程先运行结束了？所以显示出来的“最后账户余额”不正确。

Answer 1

根本原因在于，你打印余额时，其实另外两个存钱和取钱线程还没有执行完呢？你一定没注意这一点

Answer 2

因为GetMoney/SetMoney组合起来不是原子的操作. Get获取的值可能是老的值. 如果只是+1的话, 可以用fetch and add; 如果非要设计成Get/Set, 那么得用compare and swap. 你可以自行Google一下, 这些概念Java都有相应的实现.

PS:
被钓鱼了.

Answer 3

你这种情况下 money 用 AtomicInteger 来代替 Integer，AtomicInteger是线程安全的,而int不是

Answer 4

主要有两个问题。
1、如@piano 所说的， 打印语句放错地方了，要放在子线程里才对。
2、还有一个更重要的问题，就是for循环要放在同步代码块里面，如下

            synchronized (account) 

            {for(int i=0;i<50000;i++)

               {……

               }

            }

如果把synchronized语句放到for语句里面（如同提问者那样的写法），就会造成每次循环都出现“加锁->修改->解锁”的情况，共50000次，在每次解锁后到再次对account加锁期间，就会出现account被其他线程修改的情况。