Ещё раз (надеюсь, последний) про double-checked locking
Статей про 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
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
Код теста 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
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, которая предупреждает о таких ситуациях. Однако она может сработать не всегда, поэтому лучше полагайтесь на свою голову.
