Есть ли у меня проблема с переупорядочением, и это связано с побегом ссылки?

У меня есть этот класс, где я кеширую экземпляры и клонирую их (данные изменяемы), когда я их использую.

Интересно, могу ли я столкнуться с проблемой переупорядочения с этим.

Я посмотрел на этот ответ и JLS, но я все еще не уверен.

public class DataWrapper {
    private static final ConcurrentMap<String, DataWrapper> map = new ConcurrentHashMap<>();
    private Data data;
    private String name;

    public static DataWrapper getInstance(String name) {
        DataWrapper instance = map.get(name);
        if (instance == null) {
            instance = new DataWrapper(name);
        }
        return instance.cloneInstance();
    }

    private DataWrapper(String name) {
        this.name = name;
        this.data = loadData(name);  // A heavy method
        map.put(name, this);  // I know
    }

    private DataWrapper cloneInstance() {
        return new DataWrapper(this);
    }

    private DataWrapper(DataWrapper that) {
        this.name = that.name;
        this.data = that.data.cloneInstance();
    }
}

Мое мышление: среда выполнения может изменить порядок операторов в конструкторе и опубликовать текущий DataWrapper экземпляр (положить в карту) перед инициализацией data объект. Второй поток читает DataWrapper Экземпляр с карты и видит ноль data поле (или частично построено).

Это возможно? Если да, это только из-за побега по ссылке?

Если нет, то не могли бы вы объяснить, как рассуждать о согласованности " до и до" в более простых терминах?

Что делать, если я сделал это:

public class DataWrapper {
    ...
    public static DataWrapper getInstance(String name) {
        DataWrapper instance = map.get(name);
        if (instance == null) {
            instance = new DataWrapper(name);
            map.put(name, instance);
        }
        return instance.cloneInstance();
    }

    private DataWrapper(String name) {
        this.name = name;
        this.data = loadData(name);     // A heavy method
    }
    ...
}

Это все еще склонно к той же самой проблеме?

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

РЕДАКТИРОВАТЬ:

Что если поля имени и данных были окончательными или изменчивыми?

public class DataWrapper {
    private static final ConcurrentMap<String, DataWrapper> map = new ConcurrentHashMap<>();
    private final Data data;
    private final String name;
    ... 
    private DataWrapper(String name) {
        this.name = name;
        this.data = loadData(name);  // A heavy method
        map.put(name, this);  // I know
    }
    ...
}

Это все еще небезопасно? Насколько я понимаю, гарантии безопасности инициализации конструктора применяются только в том случае, если ссылка не экранирована во время инициализации. Я ищу официальные источники, которые подтверждают это.

Источник

2 ответа

Решение

Если вы хотите быть совместимым со спецификацией, вы не можете применить этот конструктор:

private DataWrapper(String name) {
  this.name = name;
  this.data = loadData(name);
  map.put(name, this);
}

Как вы указали, JVM разрешено изменить порядок на что-то вроде:

private DataWrapper(String name) {
  map.put(name, this);
  this.name = name;
  this.data = loadData(name);
}

При назначении значения final поле, это подразумевает так называемое действие замораживания в конце конструктора. Модель памяти гарантирует, что произойдет до взаимосвязи между этим действием замораживания и любой разыменовкой экземпляра экземпляра, к которому было применено это действие замораживания. Однако это отношение существует только в конце конструктора, поэтому вы нарушаете это отношение. Перетаскивая публикацию из своего конструктора, вы можете исправить это.

Если вы хотите более формальный обзор этих отношений, я рекомендую просмотреть этот набор слайдов. Я также объяснил отношения в этой презентации, начиная примерно с минуты 34.

Источник

Реализация имеет несколько очень тонких предостережений.

Кажется, вы знаете, но для ясности, в этом фрагменте кода несколько потоков могут получить null экземпляр и введите if блок, излишне создавая новый DataWrapper экземпляры:

public static DataWrapper getInstance(String name) {
    DataWrapper instance = map.get(name);
    if (instance == null) {
        instance = new DataWrapper(name);
    }
    return instance.cloneInstance();
}

Кажется, вы в порядке с этим, но это требует предположения, что loadData(name) (использован DataWrapper(String)) всегда будет возвращать одно и то же значение. Если он может возвращать разные значения в зависимости от времени, нет гарантии, что последний поток, загрузивший данные, сохранит его в mapТаким образом, значение может быть устаревшим. Если вы говорите, что этого не произойдет или это не важно, это нормально, но это предположение должно быть хотя бы документировано.

Чтобы продемонстрировать еще одну тонкую проблему, позвольте мне instance.cloneInstance() метод:

public static DataWrapper getInstance(String name) {
    DataWrapper instance = map.get(name);
    if (instance == null) {
        instance = new DataWrapper(name);
    }
    return new DataWrapper(instance);
}

Тонкая проблема здесь в том, что это заявление о возврате не является безопасной публикацией. Новый DataWrapper Экземпляр может быть частично создан, и поток может наблюдать его в несогласованном состоянии, например, поля объекта могут быть еще не установлены.

Для этого есть простое решение: если вы сделаете name а также data поля finalкласс становится неизменным. Неизменяемые классы имеют специальные гарантии инициализации, а return new DataWrapper(this); становится безопасной публикацией.

С этим простым изменением и при условии, что вы согласны с первым пунктом (loadData не чувствителен ко времени), я думаю, что реализация должна работать правильно.

Я бы порекомендовал дополнительное улучшение, которое связано не с правильностью, а с другими хорошими практиками. Текущая реализация имеет слишком много обязанностей: это оболочка Data, и в то же время кеш. Дополнительная ответственность делает чтение немного запутанным. И, кроме того, параллельная хэш-карта на самом деле не используется для своего потенциала.

Если вы разделите обязанности, результат может быть проще, лучше и легче для чтения:

class DataWrapperCache {

  private static final ConcurrentMap<String, DataWrapper> map = new ConcurrentHashMap<>();

  public static DataWrapper get(String name) {
    return map.computeIfAbsent(name, DataWrapper::new).defensiveCopy();
  }
}

class DataWrapper {

  private final String name;
  private final Data data;

  DataWrapper(String name) {
    this.name = name;
    this.data = loadData(name);  // A heavy method
  }

  private DataWrapper(DataWrapper that) {
    this.name = that.name;
    this.data = that.data.cloneInstance();
  }

  public DataWrapper defensiveCopy() {
    return new DataWrapper(this);
  }
}
Источник
Другие вопросы по тегам