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

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

2016-04-21 Thread Peter Levart
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 { private final ReadWriteLock lock = new ReentrantReadWriteLock(); private final Lock readLock = l

Re: #ReflectiveAccessByInstrumentationAgents

2016-04-21 Thread Alan Bateman
On 21/04/2016 12:09, Peter Levart wrote: : The situation is not so complicated, I think. If instrumented code calls some code in your Byteman module(s) then you should 1st make this code public and reside in exported packages. All you have to do then is add read edges from modules of instru

Re: #ReflectiveAccessByInstrumentationAgents

2016-04-21 Thread Andrew Dinn
Hi Peter, On 21/04/16 12:09, Peter Levart wrote: > The situation is not so complicated, I think. . . . Thank you very much for your example code. I understand that it will be possible to localize the changes into a small part of the agent codebase. My problem was not with this factoring of the c

Re: #ReflectiveAccessByInstrumentationAgents

2016-04-21 Thread Peter Levart
Hi Andrew, On 04/21/2016 09:31 AM, Andrew Dinn wrote: On 20/04/16 12:40, Alan Bateman wrote: On 19/04/2016 18:00, Andrew Dinn wrote: : I have been building and testing on JDK9 Byteman using the EA program releases and have not seen anything break so far. However, that may just be because i) B

Re: #ReflectiveAccessByInstrumentationAgents

2016-04-21 Thread Alan Bateman
On 21/04/2016 08:31, Andrew Dinn wrote: : Having to explicitly manage module dependencies to retain compatibiity with previous behaviour is at the very least a /nuisance/ because it now means I -- and, no doubt, others in the agent business -- will have to implement a dual-source agent, one for

Re: [9] Review request: 8153872: Nashorn no longer needs access to com.sun.javafx.application

2016-04-21 Thread Sundararajan Athijegannathan
+1 On 4/21/2016 10:19 AM, Mandy Chung wrote: >> On Apr 19, 2016, at 8:25 AM, Kevin Rushforth >> wrote: >> >> Jim, >> >> Please review the following fix: >> >> https://bugs.openjdk.java.net/browse/JDK-8153872 >> http://cr.openjdk.java.net/~kcr/8153872/webrev.00/ >> >> This is a simple backout of

Re: #ReflectiveAccessByInstrumentationAgents

2016-04-21 Thread Andrew Dinn
On 20/04/16 12:40, Alan Bateman wrote: > > On 19/04/2016 18:00, Andrew Dinn wrote: >> : >> I have been building and testing on JDK9 Byteman using the EA program >> releases and have not seen anything break so far. However, that may just >> be because i) Byteman and the Byteman tests are only usi