Всегда ли ленивая инициализация с неизменяемыми данными является потокобезопасной?

У меня есть два класса A и B:

class A {
    private final String someData;
    private B b;

    public String getSomeData() { return someData; }

    public B getB() {
        if (b == null) {
             b = new B(someData);
        }
        return b;
    }
}

где B неизменяем и вычисляет свои данные только из экземпляра A. A имеет неизменяемую семантику, но его внутренние элементы изменяемы (например, hashCode в java.lang.String).

Когда я вызываю getB() из двух разных потоков, и вызовы перекрываются, я предполагаю, что каждый поток получает свой собственный экземпляр B. Но поскольку конструктор B получает только неизменяемые данные, два экземпляра B должны быть равны.

Это правильно? Если нет, должен ли я сделать getB() синхронизированным, чтобы сделать его потокобезопасным?

Предположим, что B реализует equals(), который сравнивает все переменные экземпляра B. То же самое для hashCode()


person Christoph Walesch    schedule 09.03.2014    source источник
comment
B переопределяет equals?   -  person Mark Elliot    schedule 10.03.2014
comment
Когда два потока вызывают getB() одновременно, может случиться так, что каждый поток имеет свой собственный объект B, или также может случиться так, что они оба используют один и тот же B. Вопрос здесь такой же, как указывает @MarkElliot: B переопределяет equals() и hashCode()?   -  person morgano    schedule 10.03.2014
comment
@MarkElliot да, предположим, что B реализует equals(), который сравнивает все переменные экземпляра B. То же самое для hashCode().   -  person Christoph Walesch    schedule 10.03.2014


Ответы (2)


Это не потокобезопасно, так как вы не создали отношений "происходит до" с volatile или synchronized, поэтому два потока могут мешать друг другу.

Проблема в том, что хотя b = new B(someData) означает «выделить достаточно памяти для экземпляра B, затем создать там экземпляр, затем указать на него b», системе разрешено реализовать его как «выделить достаточно памяти для экземпляра B, затем указать b, затем создайте экземпляр" (поскольку в однопоточном приложении это эквивалентно). Таким образом, в вашем коде, где два потока могут создавать отдельные экземпляры, но возвращать один и тот же экземпляр, есть шанс, что один поток вернет экземпляр другого потока раньше экземпляра полностью инициализирован.

person ruakh    schedule 10.03.2014

Для «Но поскольку конструктор B получает только неизменяемые данные, два экземпляра B должны быть равны». Как вы понимаете, это не потокобезопасно, один поток может получить неинициализированный экземпляр B (B как нулевое или несогласованное состояние, где некоторые данные еще не установлены), другие могут получить экземпляр b с набором некоторых данных.

Чтобы исправить это, вам нужен синхронизированный метод getB или используйте синхронизированный блок с блокировкой с двойной проверкой или какой-либо неблокирующий метод, такой как AtomicReference. Для справки я добавляю сюда пример кода, как добиться правильного метода threadSafe getB() с помощью AtomicReference.

class A {
    private final String someData = "somedata";
    private AtomicReference<B> bRef;

    public String getSomeData() { return someData; }

    public B getB() {
        if(bRef.get()== null){
            synchronized (this){
                if(bRef.get() == null)
                    bRef.compareAndSet(null,new B(someData));
            }
        }
        return bRef.get();
    }
}

class B{
    public B(String someData) {

    }
}
person Mak    schedule 10.03.2014
comment
Я не решаюсь понизить это, потому что, кроме того, что он не смог инициализировать b (вероятно, его следует назвать bRef, чтобы избежать путаницы, и объявить + инициализировать как private final AtomicReference<B> bRef = new AtomicReference<B>()), ваш код не сломан . . . но это действительно странно. Во-первых, вы не должны синхронизироваться на B.class, потому что это означает, что для всех экземпляров A потребуется одна и та же блокировка, которая также может удерживаться каким-то другим кодом (поскольку A не владеет B.class). Во-вторых, compareAndSet лишнее, так как ссылка гарантированно будет null [продолжение] - person ruakh; 10.03.2014
comment
[продолжение] в этом месте; вы можете просто написать bRef.set(new B(someData)), после чего вы можете просто использовать поле volatile. Весь смысл атомарности в том, чтобы избежать блокировки; если вы когда-нибудь обнаружите, что комбинируете атомарные числа с synchronized, вы можете быть уверены, что делаете что-то неправильно. - person ruakh; 10.03.2014