请问一个 Java 线程安全问题

V2EX = way to explore

V2EX 是一个关于分享和探索的地方

现在注册

已注册用户请登录

这是一个创建于 2381 天前的主题，其中的信息可能已经有所发展或是发生改变。

public static void register(Class<? extends Event> eventClass, Subscriber subscriber) {
        CopyOnWriteArraySet<Subscriber> set = SUBSCRIBER_MAP.get(eventClass);
        if (set == null) {
            set = new CopyOnWriteArraySet<Subscriber>();
            // 这里有点意思,判断了两次是不是 null,这样能够线程安全吗?
            CopyOnWriteArraySet<Subscriber> old = SUBSCRIBER_MAP.putIfAbsent(eventClass, set);
            if (old != null) {
                set = old;
            }
        }
        set.add(subscriber);
        if (LOGGER.isDebugEnabled()) {
            LOGGER.debug("Register subscriber: {} of event: {}.", subscriber, eventClass);
        }
    }

两次判断 null,是不是从线程安全的角度考虑的

9 条回复

raynor2011

2018-05-27 17:39:48 +08:00

没法保证，old != null 和 set = old 中间一样有可能被另一个线程改写

MrXiong

2018-05-27 17:42:35 +08:00

@raynor2011 那为什么这么做呢

MrXiong

2018-05-27 17:44:26 +08:00

@raynor2011 我看了代码没有别的地方改写这个 Set

neoblackcap

2018-05-27 17:50:24 +08:00

没法保证线程安全，想安全，老实上锁

MrXiong

2018-05-27 17:50:50 +08:00

补充:map 是 ` private final static ConcurrentHashMap<Class<? extends Event>, CopyOnWriteArraySet<Subscriber>> SUBSCRIBER_MAP = new ConcurrentHashMap<Class<? extends Event>, CopyOnWriteArraySet<Subscriber>>();`

raynor2011

2018-05-27 17:51:26 +08:00

@MrXiong 两个线程同时调用 register 就可能出错

alamaya

2018-05-27 17:55:49 +08:00

ConcurrentHashMap 本来就是线程安全，这里 putIfAbsent 就是为了保证只有一个初始化的 set 被装入 map 里

raynor2011

2018-05-27 18:08:37 +08:00

@alamaya 但是 old ！= null 的判断，和把对象加入 set，这一步不是原子的，并不能保证线程安全，正确的方案应该是提供一个线程安全的 add_new 方案，这样如果之前以及初始化过了，就不会再添加一遍

MrXiong

2018-05-27 18:11:55 +08:00

@raynor2011 由于没有 remove 方法所以一旦 put 到 map 中就不会删除,所以下面的 old ！= null 的判断不需要保证安全性,只需要保证只有一个初始化的 set 放到 map 里就行,因此这个方法是没有问题的