java.lang.reflect.Module.WeakSet is not thread-safe

[Peter Levart]

On 04/21/2016 06:07 PM, Peter Levart wrote:
> 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.multithreadUnsafe/webrev.01/ 
>

Oops...

It looks like I replaced one thread-unsafe construction with another one:

  389     // additional exports added at run-time
  390     // source module (1st key), target module (2nd key), exported 
packages (value)
  391     private static final WeakPairMap<Module, Module, Set<String>> 
transientExports =
  392         new WeakPairMap<>();

...that would've been OK if I hadn't used normal HashSet for holding the 
set of packages:

  623         // add package name to transientExports if absent
  624         transientExports
  625             .computeIfAbsent(this, other, (_this, _other) -> new 
HashSet<>())
  626             .add(pn);

Luckily this can be easily fixed by using a ConcurrentHashMap instead of 
HashSet which is even more space-friendly (HashSet is just a wrapper 
around HashMap and HashMap is basically the same structure as 
ConcurrentHashMap):

http://cr.openjdk.java.net/~plevart/jdk9-dev/Module.WeakSet.multithreadUnsafe/webrev.02/


Regards, Peter