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

[Peter Levart]

Hi Alan,

Thanks for taking a look.

On 04/21/2016 10:41 PM, Alan Bateman wrote:
>
> On 21/04/2016 20:52, Rémi Forax wrote:
>> I remember seeing this codd an thinking that synchronized should do 
>> the job.
>> I don't believe this use case requires something more complex.
>>
>> Remi
> I've taken a first pass over it and WeakPairMap seems straight-forward 
> to use but its implementation, with Pair/Weak/Primary/Secondary/Lookup 
> is complex.
>
> Prior to #ReflectionWithoutReadability then transientReads was 
> important, less so now although I think we should continue to allow 
> concurrent lookups.
>
> -Alan


I tried to reduce the complexity of WeakPairMap as much as I could. I 
added some docs that describe the architecture. Hopefully this is now 
easier to grasp:

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

Another possibility for transientReads and transientExports (but not for 
transientUses) could be if each instance of Module object held a unique 
long id allocated at its creation from say AtomicLong. You could then 
construct maps with Long ids instead of Module(s), but that has a 
drawback that some Module (say a system module or a module of an app 
server) could accumulate ids from modules long gone (for example when 
some app in an app server is redeployed multiple times) so this would be 
a memory leak...

a single global WeakPairMap has an advantage over individual WeakSet(s) 
per Module instance because it is accessed more frequently so expunging 
of stale entries happens more promptly.

Regards, Peter