基于Java同步的银行模拟器

Question

我有一个方法转账()，它从一个帐户取款并存入另一个帐户。有10个帐户，每个帐户运行自己的线程。我有另一个方法测试()，它总结每个帐户的总数，以确保银行没有损失或获得资金。为了得到一个准确的总数，我创建了一个布尔标志来指示测试是否正在进行。如果是的话，我需要以某种方式暂停传输，直到测试结束。我尝试使用同步块来实现这一点，告诉线程等待条件，并在条件不再为真时释放。因为某种原因我有困难。我的转移方法如下：

public class Bank {

    public static final int NTEST = 10;
    private Account[] accounts;
    private long ntransacts = 0;
    private int initialBalance;
    private int numAccounts;
    private boolean open;
    private int transactsInProgress;
    private boolean testing=false;

    public Bank(int numAccounts, int initialBalance) {
        open = true;
        this.initialBalance = initialBalance;
        this.numAccounts = numAccounts;
        accounts = new Account[numAccounts];
        for (int i = 0; i < accounts.length; i++) {
            accounts[i] = new Account(this, i, initialBalance);
        }
        ntransacts = 0;
        transactsInProgress = 0;
    }
    public synchronized void incrementTransacts(){
        transactsInProgress++;
    }
    public synchronized void decrementTransacts(){
        transactsInProgress--;
    }

    public void transfer(int from, int to, int amount) throws InterruptedException {

    accounts[from].waitForAvailableFunds(amount);
    synchronized(this){
        while(testing){
            System.out.println("Cannot transfer while testing...");
            this.wait();
        }
    }
        if (!open) return;
        if (accounts[from].withdraw(amount)) {
            incrementTransacts(); //synchronzied method increments transactsInProgress
            accounts[to].deposit(amount);
            decrementTransacts(); //synchronized method
        }
        if (shouldTest()) test();

    synchronized(this){
        this.notifyAll();
    }    
    }

    public synchronized void test() throws InterruptedException {
        int sum = 0;

        testing=true;
        while(transactsInProgress!=0){
                System.out.println("Cannot test while transactions are in progres... \nWaiting...");
            wait();
        }

        for (int i = 0; i < accounts.length; i++) {
            System.out.printf("%s %s%n", 
                    Thread.currentThread().toString(),accounts[i].toString());
            sum += accounts[i].getBalance();
        }
        System.out.println(Thread.currentThread().toString() + 
                " Sum: " + sum);
        if (sum != numAccounts * initialBalance) {
            System.out.println(Thread.currentThread().toString() + 
                    " Money was gained or lost");
            System.exit(1);
        } else {
            System.out.println(Thread.currentThread().toString() + 
                    " The bank is in balance");
        }
        testing=false;
        notifyAll();
    }
       public int size() {
        return accounts.length;
    }

    public synchronized boolean isOpen() {return open;}

    public void closeBank() {
        synchronized (this) {
            open = false;
        }
        for (Account account : accounts) {
            synchronized(account) {
                account.notifyAll();
            }
        }
    }

    public synchronized boolean shouldTest() {
        return ++ntransacts % NTEST == 0;
    }
}

我已经有一段时间没有用Java编写代码了，我对线程和并发性还不熟悉，所以我不知道自己到底出了什么问题。当我运行程序时，银行金额是不正确的。每个帐户有10,000，所以每次的金额应该是100,000。这里有什么想法吗？

编辑:线程类和Main：

class TransferThread extends Thread {

    public TransferThread(Bank b, int from, int max) {
        bank = b;
        fromAccount = from;
        maxAmount = max;
    }

    @Override
    public void run() {
        for (int i = 0; i < 10000; i++) {
            int toAccount = (int) (bank.size() * Math.random());
            int amount = (int) (maxAmount * Math.random());
            bank.transfer(fromAccount, toAccount, amount);
        }
        bank.closeBank();
    }
    private Bank bank;
    private int fromAccount;
    private int maxAmount;
}

Main：

public static void main(String[] args) throws InterruptedException {
    Bank b = new Bank(NACCOUNTS, INITIAL_BALANCE);
    Thread[] threads = new Thread[NACCOUNTS];
    // Start a thread for each account
    for (int i = 0; i < NACCOUNTS; i++) {
        threads[i] = new TransferThread(b, i, INITIAL_BALANCE);
        threads[i].start();
    }
    // Wait for all threads to finish
    for (int i = 0; i < NACCOUNTS; i++) {
        try {
            threads[i].join();
        } catch (InterruptedException ex) {
            // Ignore this
        }
    }
    b.test();
}

Answer 1

我不知道您的确切问题，但是在您的代码中有一些关于问题的内容：

1. 您的transfer()方法有两个不同的synchronized块，但似乎要执行应该在它们之间进行保护的操作。
2. 不要信任原始boolean变量进行同步。当您使用多个线程时，您应该使用AtomicBoolean。

更新现在我更好地理解了这个问题：

这里的问题是，您使用的是试图使用synchronized的方式，而这并不是它的设计者想要的。如果要同步，就选择一个对象，并说“一次只有一个线程可以操作这个东西”。使用synchronized(this)或在Bank类中将方法声明为synchronized时，“只有一个线程可以同时操作银行的状态”。

从你下面的评论来看，我知道情况并非如此。如果多个线程可以同时更新帐户，则银行不是要同步的资源。

您应该在更细粒度的级别上进行保护(例如，分别锁定每个帐户)，或者使用不同的锁构造，例如ReadWriteLock，它允许多个线程共享更低级别的访问，或者使用单个线程获得独占访问。

Answer 2

这个bug并不是立即显现出来的，但是这看上去是错误的：

    if (accounts[from].withdraw(amount)) {
        incrementTransacts(); //synchronzied method increments transactsInProgress
        accounts[to].deposit(amount);
        decrementTransacts(); //synchronized method
    }

这部分代码不是同步的。如果两个线程试图存到同一个帐户，它们可能会覆盖对方。或者，如果没有任何内存屏障，正确的平衡可能无法正确发布。您没有显示Account对象，但是如果它不是线程安全的，这可能是问题的根源。

每个帐户都可以有一个AtomicInteger余额，然后以线程安全的方式进行原子更新。

class Account {
   private final AtomicInteger balance = new AtomicInteger(0);
   ...
   public int withdraw(int amount) {
        // loop to get the money in a thread-safe manner
        while (true) {
           // get current balance
           int current = balance.get();
           if (current < amount) {
               // not enough funds
               return 0;
           }
           // update the balance atomically if it is still current
           if (balance.compareAndSet(current, current - amount)) {
               return amount;
           }
           // someone else beat me to it so loop and get new balance
       }
   }

   public void deposit(int amount) {
       // similar but without the not-enough-funds check and with a +
   }

下面是您的代码中可能有帮助的其他一些问题。

1. Accountsfrom.waitForAvailableFunds(量)；

你在等待资金，然后取回资金。有一个竞赛条件，可能会导致另外两个线程都返回true，但只有一个线程才能实际获得资金。我们也无法看到Account类以确保它是线程安全的。

我会有某种accounts[from].waitAndWithdraw(...)方法，它会等到帐户有了资金，然后取款。

1. 在我看来，如果得到正确的取款和存款命令组合，您可以很容易地锁定系统。例如，如果发生了10次撤资，他们中的任何一个都可能没有资金，而你的系统就会停止。您可以使用超时值执行wait(...)，并在到达超时时返回错误代码(或抛出)。 这种情况不会发生在您编写的main(...)中，但是其他用户可以这样做。
2. public synchronized void decrementTransacts(){ transactsInProgress--; } 这类代码正在乞求使用AtomicInteger。然后，您可以不使用同步锁执行transactsInProgress.incrementAndGet()。
3. 正如@泰勒所提到的，您可以考虑在Account对象上而不是在Bank上同步。因此，Bank只是发出传输的类，但是锁将本地化在Account对象上，以确保它们是线程安全的。或者使用上面的代码，许多同步锁可能是不必要的。
4. 很明显，有些时候test()会失败，因为有时您已经从一个帐户中提取了一个金额，但可能没有将相同的金额存入另一个帐户。