Безопасно ли использовать writeLock без readlock?

Question

Предполагая, что это "одиночная" реализация: я гарантирую, что это будет вызывать productCatalogLoader.load () только один раз, а также что нулевые указатели не могут произойти? Есть ли способ сделать это проще?

private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
private ProductCatalog productCatalog;

public ProductCatalog get() {
        if (this.productCatalog == null) {
            reload();
         }
        return this.productCatalog;
}                                                         

public void reload() {

    lock.writeLock().lock();
    try {
        if (this.productCatalog != null) return;
        this.productCatalog = productCatalogLoader.load();
    } finally {
        lock.writeLock().unlock();
    }
}

}

Edit: Это была довольно успешная попытка свести гораздо более сложный код к простому образцу вопросов. Несколько человек поняли, что мой синглтон был слишком сложным;) (На самом деле есть также кварцевый таймер, вызывающий перезагрузку, но реализация «перезагрузки» - это немного другой IRL). В любом случае я получил ответ, который мне был нужен, сломанная блокировка с двойной проверкой.

Answer 1

Лучший способ упростить его, IMO, - это использовать «нормальную» блокировку, если у вас нет убедительных доказательств того, что она вызывает узкое место:

private final Object lock = new Object();
private ProductCatalog productCatalog;

public ProductCatalog get() {
    synchronized (lock) {
        if (this.productCatalog == null) {
            this.productCatalog = productCatalogLoader.load();
        }
        return this.productCatalog;
    }
}                                                         

В подавляющем большинстве случаев этого будет достаточно, и вам не нужно беспокоиться о технических особенностях модели памяти.

РЕДАКТИРОВАТЬ: относительно того, является ли чтение данных, измененных в блокировке записи без получения блокировки чтения, безопасно - я подозреваю нет, но я не хотел бы сказать наверняка. Есть ли какая-то польза в вашем подходе к использованию обычной (и безопасной на Java 1.5+, если вы осторожны) двойной проверки блокировки с использованием переменной volatile?

private final Object lock = new Object();
private volatile ProductCatalog productCatalog;

public ProductCatalog get() {
    if (this.productCatalog == null) {
        synchronized (lock) {
            if (this.productCatalog == null) {
                this.productCatalog = productCatalogLoader.load();
            }
        }
    }
    return this.productCatalog;
}                                                         

Я полагаю, что это ленивая методика инициализации, рекомендуемая в Effective Java 2nd edition, для ситуаций, когда статических инициализаторов недостаточно. Обратите внимание, что productCatalog должен быть нестабильным, чтобы это работало - и я думаю, что это эффективно , чего вам не хватает, не снимая блокировку чтения в вашем коде.

Answer 2

Нет, в общем, это небезопасно ... Блокировка записи только гарантирует, что никто другой не сможет получить блокировку чтения или записи, пока блокировка записи не будет снята. Не выполнять блокировку чтения в вашем коде - это все равно, что синхронизировать метод перезагрузки, но не метод get. Я вроде согласен с Джоном Скитом, но это во многом зависит от использования.

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

Редактировать: Кстати, есть довольно хороший пример того, как это сделать в Effective Java 2nd Edition ... Я также должен исправить себя здесь, учитывая контекст, синхронизация будет работать так же хорошо, если она будет сделана умным способом (и блокировка записи, вероятно, тоже будет работать в этом контексте).

Answer 3

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

скажем, у нас есть

var foo = new ProductCatalog();
foo.blah = "blah blah";
this.productCatalog = foo;

можно изменить как

var foo = new ProductCatalog();
this.productCatalog = foo;
foo.blah = "blah blah";

Answer 4

Разве не проще, если вы пытаетесь сделать синглтон:

private final ProductCatalog productCatalog = productCatalogLoader.load();
public ProductCatalog get() {
   return this.productCatalog;
}

Если есть какая-то причина, по которой вы не можете сделать ProductCatalog конечным членом, то либо использование синхронизированного ответа Jon Skeet, либо ваш исходный код должен работать, пока к элементу productCatalog не осуществляется доступ, кроме как через вызов get () или ваша перезагрузка ().