Статей про 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, которая предупреждает о таких ситуациях. Однако она может сработать не всегда, поэтому лучше полагайтесь на свою голову.
Автор: lany
