Безопасен ли этот код с двойной проверкой?

Question

Я смотрю на некоторый код в нашем приложении, который, как мне кажется, может встретить случай " Двойная проверка блокировки " Я написал пример кода, который похож на то, что мы делаем.

Может кто-нибудь увидеть, как это может происходить с двойной проверкой блокировки? Или это безопасно?

class Foo {
    private Helper helper = null;
    public Helper getHelper() {
        Helper result;
        synchronized(this) {
            result = helper;
        }

        if (helper == null) {
            synchronized(this) {
                if (helper == null) {
                    helper = new Helper();
                }
            }
        }
        return helper;
    }
}

Базовый код заимствован из вики .

Answer 1

Это излишне сложно, самый простой и безопасный способ сделать DCL выглядит так:

class Foo {
  private volatile Helper helper = null;
  private final Object mutex = new Object(); 
  public Helper getHelper() {
    if (helper == null) {
        synchronized(mutex) {
            if (helper == null) {
                helper = new Helper();
            }
        }
    }
    return helper;
  }
}

Ключевые моменты здесь:

• В «счастливом» случае мы ожидаем, что помощник уже назначен, поэтому, если это так, мы можем просто вернуть его, не вводя синхронизированный блок.
• Помощник помечен как энергозависимый, чтобы сообщить компилятору, что помощник может быть прочитан / записан любым потоком в любое время, и важно, чтобы чтение / запись не переупорядочивались.
• Синхронизированный блок использует частную конечную переменную для синхронизации, чтобы избежать потенциального снижения производительности в случае другой области синхронизации кода на экземпляре this.

Answer 2

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

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

Answer 3

Эта часть представляет собой стандартную блокировку с двойной проверкой:

if (helper == null) {
    synchronized(this) {
        if (helper == null) {
            helper = new Helper();
        }
    }
}

Первая часть - бесполезное присвоение, которое ничего не делает с блокируемой частью с двойной проверкой: если хелпер имеет значение null, то он выполняется в любом случае,если это не так, это не будет выполнено в любом случае.Это совершенно неэффективно.