Безопасен ли этот поток Java-класса? - PullRequest
10 голосов
/ 23 января 2012

Это не домашнее задание для меня , это задание для студентов из какого-то университета. Я заинтересован в решении из личного интереса.

Задача - создать класс (Calc), который содержит целое число. Два метода add и mul должны добавлять или умножать это целое число.

Два потока настраиваются одновременно. Один поток должен вызывать c.add (3) десять раз, другой должен вызывать c.mul (3) десять раз (конечно, для того же объекта Calc).

Класс Calc должен убедиться, что операции выполняются поочередно (add, mul, add, mul, add, mul, ..).

Я не слишком много работал с проблемами, связанными с параллелизмом, а тем более с Java. Я придумал следующую реализацию для Calc:

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: подпись методов не должна быть изменена. Кроме того, я не ограничен.

Ответы [ 7 ]

11 голосов
/ 23 января 2012

Попробуйте использовать интерфейс блокировки:

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(), если другой поток удерживает lock, пока не будет вызван .signal().

10 голосов
/ 23 января 2012

То, что вы сделали, это занятый цикл: вы запускаете цикл, который останавливается только при изменении переменной. Это плохая методика, поскольку она сильно загружает процессор, а не заставляет поток ждать, пока флаг не изменится.

Я бы использовал два семафора : один для 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();
    }
}
5 голосов
/ 23 января 2012

Как уже говорили другие, volatile в вашем решении требуется.Кроме того, ваше решение вращается, что может потратить довольно много циклов ЦП.Тем не менее, я не вижу никаких проблем в том, что касается правильности.

Я лично реализовал бы это с парой семафоров:

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();
}
3 голосов
/ 23 января 2012

Хммм. Есть несколько проблем с вашим решением. Во-первых, энергозависимость требуется не для атомарности, а для видимости. Я не буду вдаваться в подробности, но вы можете прочитать больше о модели памяти Java . (И да, логическое значение атомарно, но здесь оно не имеет значения). Кроме того, если вы обращаетесь к переменным только внутри синхронизированных блоков, то они не должны быть энергозависимыми.

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

Идиоматическое решение будет выглядеть так:

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();
    }
}
3 голосов
/ 23 января 2012
Требуется

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 проверяется внутри блока синхронизации) энергозависимо не требуется

3 голосов
/ 23 января 2012

Да, 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, чтобы освободить первый поток.

0 голосов
/ 23 января 2012

Программа полностью поточно-ориентированная:

  1. Логический флаг установлен на volatile, поэтому JVM знает, что не нужно кэшировать значения и сохранять доступ на запись к одному потоку за раз.

  2. Две критические секции блокируют текущий объект, что означает, что только один поток будет иметь доступ одновременно.Обратите внимание, что если поток находится внутри синхронизированного блока, он не может быть ни в каких других критических секциях.

Вышеуказанное будет применяться к каждому экземпляру класса.Например, если созданы два экземпляра, потоки смогут одновременно входить в несколько критических разделов, но будут ограничены одним потоком на экземпляры на критический раздел.Имеет ли это смысл?

...