Pull to refresh

Ещё раз (надеюсь, последний) про double-checked locking

Reading time 4 min
Views 51K
Статей про double-checked locking на Хабре было столько, что казалось бы ещё одна — и Хабр лопнет. Вот только по Java неплохие публикации: Реализация Singleton в JAVA, Правильный Singleton в Java, А как же всё-таки работает многопоточность? Часть II: memory ordering или вот замечательный пост от TheShade (слава web-archive!). В наши дни, наверно, каждый Java-разработчик слышал, что если используешь DCL, будь добр объявить переменную volatile. Найти сегодня в коде известных опенсорсных проектов DCL без volatile довольно трудно, но оказалось, что проблемы ещё не полностью решены. Поэтому я добавлю небольшую заметку по теме с примерами из реальных проектов.

Иногда складывается ощущение, что программисты не включают мозги и не пытаются понять, как что работает, а просто следуют простым и понятным правилам вроде «объяви переменную volatile, используй DCL, и всё будет хорошо». К сожалению, такой подход в программировании не всегда работает.

Особенность DCL-паттерна в том, что момент публикации объекта — это операция volatile-записи, а не выход из секции синхронизации. Поэтому именно volatile-запись должна производиться после полной инициализации объекта.

Вот, к примеру, такой код обнаружился в проекте ICU4J — TZDBTimeZoneNames#prepareFind:
    private static volatile TextTrieMap<TZDBNameInfo> TZDB_NAMES_TRIE = null;

    private static void prepareFind() {
        if (TZDB_NAMES_TRIE == null) {
            synchronized(TZDBTimeZoneNames.class) {
                if (TZDB_NAMES_TRIE == null) {
                    // loading all names into trie
                    TZDB_NAMES_TRIE = new TextTrieMap<TZDBNameInfo>(true);
                    Set<String> mzIDs = TimeZoneNamesImpl._getAvailableMetaZoneIDs();
                    for (String mzID : mzIDs) {
                        ...
                        TZDB_NAMES_TRIE.put(std, stdInf);
                        ...
                    }
                }
            }
        }
    }

Разработчик написал volatile, потому что где-то слышал, что так надо, но, видимо, не понял, зачем. По факту публикация объекта TZDB_NAMES_TRIE состоялась в момент volatile-записи: после этого вызовы prepareFind в других потоках будут сразу выходить без синхронизации. При этом после публикации производится множество дополнительных шагов по инициализации.

Данный метод используется при поиске часового пояса, и этот поиск вполне можно сломать. В нормальных условиях new TZDBTimeZoneNames(ULocale.ENGLISH).find("GMT", 0, EnumSet.allOf(NameType.class)) должен выдавать один результат. Выполним этот код в 1000 потоков:
Код теста
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import com.ibm.icu.impl.TZDBTimeZoneNames;
import com.ibm.icu.text.TimeZoneNames.NameType;
import com.ibm.icu.util.ULocale;

public class ICU4JTest {
	public static void main(String... args) throws InterruptedException {
		final TZDBTimeZoneNames names = new TZDBTimeZoneNames(ULocale.ENGLISH);
		final AtomicInteger notFound = new AtomicInteger();
		final AtomicInteger found = new AtomicInteger();
		List<Thread> threads = new ArrayList<>();
		
		for(int i=0; i<1000; i++) {
			Thread thread = new Thread() {
				@Override
				public void run() {
					int resultSize = names.find("GMT", 0, EnumSet.allOf(NameType.class)).size();
					if(resultSize == 0)
						notFound.incrementAndGet();
					else if(resultSize == 1)
						found.incrementAndGet();
					else
						throw new AssertionError();
				}
			};
			thread.start();
			threads.add(thread);
		}
		for(Thread thread : threads) thread.join();
		System.out.println("Not found: "+notFound);
		System.out.println("Found: "+found);
	}
}

Результат всегда разный, но примерно такой:
Exception in thread "Thread-383" java.util.ConcurrentModificationException
	at java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:953)
	at java.util.LinkedList$ListItr.next(LinkedList.java:886)
	at com.ibm.icu.impl.TextTrieMap$Node.findMatch(TextTrieMap.java:255)
	at com.ibm.icu.impl.TextTrieMap.find(TextTrieMap.java:100)
	at com.ibm.icu.impl.TextTrieMap.find(TextTrieMap.java:89)
	at com.ibm.icu.impl.TZDBTimeZoneNames.find(TZDBTimeZoneNames.java:133)
	at a.ICU4JTest$1.run(ICU4JTest.java:23)
Exception in thread "Thread-447" java.lang.ArrayIndexOutOfBoundsException: 1
	at com.ibm.icu.impl.TextTrieMap$Node.matchFollowing(TextTrieMap.java:316)
	at com.ibm.icu.impl.TextTrieMap$Node.findMatch(TextTrieMap.java:260)
	at com.ibm.icu.impl.TextTrieMap.find(TextTrieMap.java:100)
	at com.ibm.icu.impl.TextTrieMap.find(TextTrieMap.java:89)
	at com.ibm.icu.impl.TZDBTimeZoneNames.find(TZDBTimeZoneNames.java:133)
	at a.ICU4JTest$1.run(ICU4JTest.java:23)
Not found: 430
Found: 568
Почти половина потоков ничего не нашла, пара потоков вообще упала с исключением. Да, в реальном приложении такое маловероятно, но если сценарий с высокой конкуррентностью авторов не интересует, тогда можно вообще обойтись без volatile и DCL.

Вот другой пример от IntelliJ IDEA — FileEditorManagerImpl#initUI:
  private final Object myInitLock = new Object();
  private volatile JPanel myPanels;
  private EditorsSplitters mySplitters;

  private void initUI() {
    if (myPanels == null) {
      synchronized (myInitLock) {
        if (myPanels == null) {
          myPanels = new JPanel(new BorderLayout());
          myPanels.setOpaque(false);
          myPanels.setBorder(new MyBorder());
          mySplitters = new EditorsSplitters(this, myDockManager, true);
          myPanels.add(mySplitters, BorderLayout.CENTER);
        }
      }
    }
  }

  public JComponent getComponent() {
    initUI();
    return myPanels;
  }

  public EditorsSplitters getMainSplitters() {
    initUI();
    return mySplitters;
  }

Тут авторы даже сделали приватный lock-объект для надёжности, но код всё равно сломан. Конечно, может быть ничего страшного не произойдёт, если начать пользоваться myPanels с неустановленным border, но тут проблема серьёзнее: DCL выполняется на одной переменной (myPanels), а инициализируется две (ещё и mySplitters), причём опять же volatile-запись происходит перед полной инициализацией. В результате getMainSplitters() при конкуррентном доступе вполне может вернуть null.

Исправляются такие вещи очень легко: надо сохранить объект в локальную переменную, с её помощью вызвать все необходимые методы для инициализации, а уже потом записать volatile-поле.

Ещё пара подозрительных мест:
Tomcat DBCP2 BasicDataSource: возможно увидеть объект dataSource, у которого не установлен logWriter.
Apache Wicket Application#getSessionStore(): возможно увидеть sessionStore с незарегистрированным listener'ом.

Здесь вряд ли возможны реальные проблемы, но всё равно стоит писать аккуратнее.

Я добавил небольшую эвристическую проверку в FindBugs, которая предупреждает о таких ситуациях. Однако она может сработать не всегда, поэтому лучше полагайтесь на свою голову.
Tags:
Hubs:
+44
Comments 116
Comments Comments 116

Articles