концепция синхронизации в java не работает?

У нас есть сто счетов в банке и два клерка, реализованные в виде потоков, которые переводят каждые 1000 раз деньги со счета с номером accountNumberFrom на счет accountNumberTo, используя синхронизированный метод transferMoney. Поскольку все счета начинаются с баланса 0, а деньги, полученные с одного счета, переводятся на другой, баланс после всех транзакций должен быть равен нулю. Это верно в большинстве случаев, но не всегда. Хоть это и бывает редко, но иногда баланс после транзакций не равен 0. Что не так?

public class Clerk extends Thread {
    private Bank bank;

    public Clerk(String name, Bank bank) {
        super(name);
        this.bank=bank;
        start();
    }

    public void run() {
        for (long i=0; i<1000; i++) {
            int accountNumberFrom = (int) (Math.random()*100);
            int accountNumberTo = (int) (Math.random()*100);
            float amount = (int) (Math.random()*1000) - 500;
            bank.transferMoney(accountNumberFrom, amount);
            bank.transferMoney(accountNumberTo, -amount);
        }
    }
}

and a class Bank

public class Bank {
    Account[] account;

    public Bank() {
        account = new Account[100];
        for (int i=0; i < account.length; i++)
            account[i] = new Account();
    }

    public synchronized void transferMoney(int accountNumber, float amount) {
        float oldBalance = account[accountNumber].getBalance();
        float newBalance = oldBalance + amount;
        account[accountNumber].setBalance(newBalance);
    }
}

public class Banking {
    public static void main (String[] args) {
        Bank myBank = new Bank();
        /**
         * balance before transactions
         */
        float sum=0;
        for (int i=0; i<100; i++)
            sum+=myBank.account[i].getBalance();
        System.out.println("before: " + sum);

        new Clerk ("Tom", myBank);
        new Clerk ("Dick", myBank);        

        /**
         * balance after transactions
         */
        for (int i=0; i<100; i++)
            sum+=myBank.account[i].getBalance();

        System.out.println("after: " + sum);
    }
}

java multithreading synchronize
person user40471    schedule 23.12.2019    source источник
comment
transferMoney может быть synchronized, но ваш доступ к myBank.account[i] — нет. И потоки Clerk могут все еще работать в это время; вы, вероятно, хотели join() их, прежде чем суммировать общий баланс.   -  person Thomas    schedule 23.12.2019

Ответы (3)


arrow_upward
1
arrow_downward

Одна из проблем заключается в том, что синхронизированный метод transferMoney принимает только одну учетную запись, поэтому возможно, что другой поток может получить доступ к балансу учетной записи после суммы перевода, которая была добавлена ​​к учетной записи «на», но до оно было списано со счета "от". Если бы все счета начинались с нуля, у нас могла бы быть следующая последовательность событий:

  1. Клерк Том добавляет 100 долларов на счет 1.
  2. Основной поток суммирует остатки на счетах.
  3. Клерк Том вычитает 100 долларов со счета 2.

На шаге 2 мы видим, что сумма всех счетов составляет 100 долларов вместо нуля.

Поэтому важно, чтобы метод transferMoney обновлял обе учетные записи, удерживая синхронизированную блокировку.

Другая проблема заключается в том, что, хотя transferMoney синхронизирован, код, который суммирует остатки на счетах (шаг 2 выше), не синхронизирован. Таким образом, даже если вы обновите обе учетные записи в методе transferMoney, описанная выше последовательность событий все равно может произойти, потому что основной поток не синхронизируется до выполнения шага 2.

Я бы переместил код, суммирующий учетные записи, в Bank и тоже синхронизировал. Это сделает оба метода синхронизированными в экземпляре Bank и предотвратит последовательность событий, которая приводит к неправильной ошибке.

Второстепенная проблема заключается в том, что в основном потоке вы не ждете, пока клерки закончат свои передачи. Ваш код выполняет все 1000 переводов, но вы просто проверяете балансы сразу после того, как запускаете потоки клерка, так что вы можете смотреть на балансы после 0 переводов, или после всех 1000, или после 639 переводов, кто знает. Правильное выполнение синхронизации не позволит вам увидеть ненулевой общий баланс, но вы все равно должны дождаться завершения клерков. (Попробуйте, и если вы не можете понять это, опубликуйте новый вопрос.)

person Willis Blackburn    schedule 23.12.2019

arrow_upward
0
arrow_downward

В вашем примере synchronized блокирует только вызов всех потоков в myBank.transferMoney, но это не гарантирует, что все потоки выполняются в main thread, вы можете обновить исходный код следующим образом:

class Clerk extends Thread {
    private Bank bank;
    private volatile boolean done;

    public Clerk(String name, Bank bank) {
        super(name);
        this.done = false;
        this.bank=bank;
        start();
    }

    public void run() {
        for (long i=0; i<1000; i++) {
            int accountNumberFrom = (int) (Math.random()*100);
            int accountNumberTo = (int) (Math.random()*100);
            float amount = (int) (Math.random()*1000) - 500;
            bank.transferMoney(accountNumberFrom, amount);
            bank.transferMoney(accountNumberTo, -amount);
        }
        this.done = true;
    }

    public boolean isDone() {
        return done;
    }
}

class Account {

    protected float balance;

    public float getBalance() {
        return balance;
    }

    public void setBalance(float newBalance) {
        this.balance = newBalance;
    }

}

class Bank {
    Account[] account;

    public Bank() {
        account = new Account[100];
        for (int i=0; i < account.length; i++)
            account[i] = new Account();
    }

    public synchronized void transferMoney(int accountNumber, float amount) {
        float oldBalance = account[accountNumber].getBalance();
        float newBalance = oldBalance + amount;
        account[accountNumber].setBalance(newBalance);
    }
}

public class Banking {
    public static void main (String[] args) throws Exception {
        for(int j = 0 ; j < 1000 ; ++j) {
            Bank myBank = new Bank();
            /**
             * balance before transactions
             */
            float sum=0;
            for (int i=0; i<100; i++)
                sum+=myBank.account[i].getBalance();
            System.out.println("before: " + sum);

            Clerk a = new Clerk ("Tom", myBank);
            Clerk b = new Clerk ("Dick", myBank);

            while(!a.isDone() || !b.isDone()) // wait util all thread done
                Thread.sleep(1);

            /**
             * balance after transactions
             */
            for (int i=0; i<100; i++)
                sum+=myBank.account[i].getBalance();

            System.out.println("after: " + sum);
        }
    }
}
person dung ta van    schedule 23.12.2019
comment
Нет необходимости в готовом флаге. Основной поток может все Thread.join в двух экземплярах Clerk. - person Willis Blackburn; 24.12.2019
comment
Это другой вариант, :) - person dung ta van; 24.12.2019

arrow_upward
0
arrow_downward

Большое спасибо за полезные ответы. Я изменил свой код, и теперь он работает как надо:

public class Bank
{
    Account[] account;

    public Bank() {
        account = new Account[100];
        for (int i=0; i < account.length; i++)
            account[i] = new Account();
    }

    public void transferMoney(int accountNumber, float amount) {
        synchronized (account[accountNumber]) {
            float oldBalance = account[accountNumber].getBalance();
            float newBalance = oldBalance - amount;
            account[accountNumber].setBalance(newBalance);
        }
    }
}

public class Account {
    private float balance;

    public void setBalance(float balance) {
        this.balance=balance;
    }

    public float getBalance() {
        return this.balance;
    }
}

public class Clerk extends Thread {
    private Bank bank;

    public Clerk(String name, Bank bank) {
        super(name);
        this.bank=bank;
    }

    public void run() {
        for (long i=0; i<100; i++) {
            int accountNumberFrom = (int) (Math.random()*100);
            int accountNumberTo = (int) (Math.random()*100);
            float amount = (int) (Math.random()*1000);
            bank.transferMoney(accountNumberFrom, -amount);
            bank.transferMoney(accountNumberTo, amount);
        }
    }
}

public class Accountant extends Thread
{
    Bank bank;

    public Accountant(String name, Bank bank)
    {
        super(name);
        this.bank=bank;
    }

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

    public synchronized void getBalance() {
        float sum=0;

        System.out.println(Thread.currentThread().getName());
        for (int i=0; i<100; i++)
            sum+=bank.account[i].getBalance();

        System.out.println("Bilanz: " + sum);
    }
}

public class Banking {

    public Banking() {
    }

    public static void main(String[] args) {
        Bank myBank = new Bank();
        Clerk tom = new Clerk ("Tom", myBank);
        Clerk dick = new Clerk ("Dick", myBank);        
        Accountant harry = new Accountant("Harry", myBank);

        tom.start();
        dick.start();

        try { 
            System.out.println("Current Thread: " + Thread.currentThread().getName()); 
            tom.join(); 
            dick.join(); 
        } 
        catch(Exception x) { 
            System.out.println("Exception has " + "been caught" + x); 
        }

        harry.start();
    }
}
person user40471    schedule 25.12.2019