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

2016-04-24 Thread Alan Bateman
On 22/04/2016 14:42, Peter Levart wrote: : 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/

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

2016-04-22 Thread 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

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

2016-04-21 Thread Alan Bateman
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 Pai

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

2016-04-21 Thread Rémi Forax
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 Le 21 avril 2016 18:21:55 CEST, Alan Bateman a écrit : > > >On 21/04/2016 17:07, Peter Levart wrote: >> : >> >> ...while this seems OK from 1st look,

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

2016-04-21 Thread 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

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

2016-04-21 Thread Alan Bateman
On 21/04/2016 17:07, Peter Levart wrote: : ...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 c