Это сломанная двойная проверка блокировки? - PullRequest
5 голосов
/ 01 декабря 2008

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 голосов
/ 01 декабря 2008

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

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

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

4 голосов
/ 01 декабря 2008

Если вы хотите, чтобы эта самая внутренняя строка читалась:

row = dao().create(id);

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

Редактировать : (код был обновлен)

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

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

3 голосов
/ 01 декабря 2008

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

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

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

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

2 голосов
/ 02 декабря 2008

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

  1. Java-код не сгруппирован (см. Ответ Грега Х)
  2. Ссылка на "строку": только , проверяется на нулевое значение в первой строке перед блоком синхронизации.

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

...