Это сломанный двойной проверенный замок?


Checkstyle сообщает об этом коде как о "сломанной идиоме блокировки с двойной проверкой", но я не думаю, что на мой код действительно влияют проблемы с блокировкой с двойной проверкой.

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

Псевдокод:

private void createRow(int id) {
  Row row = dao().fetch(id);
  if (row == null) {
     synchronized (TestClass.class) {
        row = dao().fetch(id);
        if (row == null) {
           dao().create(id);
        }
     }
  }
}
Я могу согласиться, что это выглядит как дважды проверенный замок, но это не так использование статических переменных и кода в fetch () и create (), вероятно, слишком сложно, чтобы быть встроенным и выведенным из строя.

Я ошибаюсь или проверяю стиль? :)

4 5

4 ответа:

Предполагая, что вы хотите, чтобы эта самая внутренняя строка гласила:

row = dao().create(id);

Это не классическая проблема блокировки с двойной проверкой, предполагающая, что dao().fetch Правильно мутируется из метода create.

Edit : (код был обновлен)

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

Предполагая, что DAO правильно синхронизирован и не возвращает частично инициализированное значение, это не страдает от недостатков идиомы двойного замка.

Я думаю, что в этом случае checkstyle является правильным. В представленном коде рассмотрим, что произойдет, если два потока будут иметь row == null на входе в синхронизированный блок. Поток а войдет в блок и вставит новую строку. Затем, после того как поток A выйдет из блока, поток B войдет в блок (потому что он не знает, что только что произошло) и попытается вставить ту же новую строку снова.

Я вижу, вы только что изменили код и добавили довольно важную недостающую строку. В новый код, Возможно, вам это сойдет с рук, поскольку два потока не будут полагаться на изменения общей (статической) переменной. Но вам лучше посмотреть, поддерживает ли ваша СУБД такой оператор, как INSERT OR UPDATE.

Еще одна веская причина делегировать эту функциональность СУБД - необходимость развертывания нескольких серверов приложений. Поскольку блоки synchronized не работают на разных машинах, вам все равно придется делать что-то другое.

Если вы испытываете искушение написать такой код, подумайте:

  • Начиная с Java 1.4, методы синхронизации стали довольно дешевыми. Это не бесплатно, но среда выполнения действительно не страдает так сильно, что стоит рисковать повреждением данных.

  • Начиная с Java 1.5, у вас есть классы Atomic*, которые позволяют вам читать и устанавливать поля атомарным способом. К сожалению, они не решают вашу проблему. Почему они не добавили AtomicCachedReference или что-то еще (что называется переопределяемый метод, когда вызывается get() и текущее значение = = null) находится вне меня.

  • Попробуйте ehcache . Он позволяет настроить кэш (т. е. И объект, который позволяет вызывать код, если ключ не содержится в карте). Обычно это то, чего вы хотите, и кэши действительно решают вашу проблему (и все те другие проблемы, о существовании которых вы даже не подозревали).

Как указывали другие, этот код будет делать то, что вы намереваетесь, как есть, но только при строгом наборе неочевидных предположений:

  1. код Java некластеризован (см. ответ @Greg H)
  2. ссылка "строка" является только проверяется на null в первой строке, перед блоком синхронизации.

Причина, по которой дважды проверенная идиома блокировки нарушена (в соответствии с разделом 16.2.4 параллелизма Java на практике), заключается в том, что это возможно для потока запуск этого метода, чтобы увидеть ненулевую, нонеправильно инициализированную ссылку на "строку", перед входом в синхронизированный блок (если только" dao " не обеспечивает правильную синхронизацию). Если бы ваш метод делал что-нибудь с "row", кроме проверки, что он null или нет, он был бы сломан. В нынешнем виде это, вероятно, хорошо, но очень хрупко - лично мне было бы неудобно совершать этот код, если бы я думал, что есть хотя бы отдаленный шанс, что какой-то другой разработчик в какой-то момент позже время может изменить метод, не понимая тонкостей DCL.