AbstractRegistryFactory have a piece of redundant code

[jingui123]

I have found a piece of redundant code, may be is I do not understand.
in AbstractRegistryFactory class, there are two variables，one is LOCK，the other is REGISTRIES,as follows :
private static final ReentrantLock LOCK = new ReentrantLock();

// Registry Collection Map<RegistryAddress, Registry>
private static final Map<String, Registry> REGISTRIES = new ConcurrentHashMap<String, Registry>();

but when we use REGISTRIES.put() , the section of code is locked. so I think REGISTRIES will be better to modify HashMap.
example:
public Registry getRegistry(URL url) {
....
LOCK.lock();
REGISTRIES.put(key, registry);
LOCK.unlock();
}
public static void destroyAll() {
....
LOCK.lock();
REGISTRIES.clear();
LOCK.unlock();
}

Environment

• Dubbo version: 2.7.0-SNAPSHOT
• Java version: jdk1.8

[carryxyh]

Hi, would u pls send a pr to fix this?

[zonghaishang]

// Lock the registry access process to ensure a single instance of the registry

[carryxyh]

Yes, but seems like the REGISTRIES field could change to hashmap.
How do u think about it? @zonghaishang

[lllichen]

public static Collection<Registry> getRegistries() { return Collections.unmodifiableCollection(REGISTRIES.values()); }
is REGISTRIES.values() escape ?

[carryxyh]

I know.
But hashmap.values is also thread safe.

[jingui123]

so，do the issue need to be modified to hashmap?

[carryxyh]

Hi, @jingui123
U can send a pr and let more people review it. They will give their suggestions.
:)

[jingui123]

ok, i will try it

[carryxyh]

Thx for your issue.