Hi, While browsing code in java.lang.reflect.Module (I sometimes do that just to see how thinks work ;-) I stumbled on the following nested class: private static class WeakSet<E> { private final ReadWriteLock lock = new ReentrantReadWriteLock(); private final Lock readLock = lock.readLock(); private final Lock writeLock = lock.writeLock(); private final WeakHashMap<E, Boolean> map = new WeakHashMap<>(); /** * Adds the specified element to the set. */ void add(E e) { writeLock.lock(); try { map.put(e, Boolean.TRUE); } finally { writeLock.unlock(); } } /** * Returns {@code true} if this set contains the specified element. */ boolean contains(E e) { readLock.lock(); try { return map.containsKey(e); } finally { readLock.unlock(); } } } ...while this seems OK from 1st look, it is not. WeakHashMap is not thread-safe even for seemingly read-only operations. All its operations can mutate internal state in a non-thread-safe way. The simplest way to fix this is to use a writeLock for containsKey operation too. But such structure does not scale well to multiple threads for frequent lookups. WeakSet is used in Module to keep track of transient read edges, exports and uses of services added dynamically to the module. Modification operations are not so performance critical, but lookup operations are. I propose to add a thread-safe WeakPairMap data structure which associates a pair of weakly-reachable keys with a strongly-reachable value based on ConcurrentHashMap. Such data structure is footprint-friendly, since only a single instance exists for a particular purpose, totaling 3 instances for the transient structures serving all Modules in the system: http://cr.openjdk.java.net/~plevart/jdk9-dev/Module.WeakSet.multithreadUnsaf... So, what do you think? Regards, Peter