Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-05-19 Thread mandy chung
Hi Peter, Sorry for the late reply.  I was on vacation last week and just returned. On 5/14/18 8:43 AM, Peter Levart wrote: Hi Mandy, Sorry for noticing this too late, but... If it was not necessary to keep legacy hacky behavior (to honor the patched "modifiers" field), wouldn't it be

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-05-14 Thread Alan Bateman
On 14/05/2018 16:43, Peter Levart wrote: : Is it really that important to allow users to modify static final fields that way? As such fields are normally constant folded by JIT, I doubt that anybody is doing it nowadays. Doing it is bound to unpredictable program behavior, as JVM is free to

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-05-14 Thread Peter Levart
On 05/11/2018 06:09 PM, mandy chung wrote: On 4/30/18 10:21 AM, Alan Bateman wrote: The updated webrev looks good. A minor comment is that I assume you can remove the cast from Executable::declaredAnnotations if you leave Executable::getRoot in place. It could but leave it as is.  I

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-05-11 Thread Alan Bateman
On 11/05/2018 17:09, mandy chung wrote: It could but leave it as is.  I found that this change breaks the hack that uses reflection to change a static final field by changing the private modifiers field in the Field object. That is a terrible hack but I think it's better to separate this

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-05-11 Thread mandy chung
On 4/30/18 10:21 AM, Alan Bateman wrote: The updated webrev looks good. A minor comment is that I assume you can remove the cast from Executable::declaredAnnotations if you leave Executable::getRoot in place. It could but leave it as is.  I found that this change breaks the hack that

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-30 Thread Alan Bateman
On 30/04/2018 17:29, mandy chung wrote: : The 3 x getRoot methods on ReflectAccess looks okay. An alternative would to create T getRoot(T obj) and a package private getRoot() method on AccessibleObject. Good idea.  I updated the patch. The updated webrev looks good. A minor comment

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-30 Thread mandy chung
On 4/30/18 7:39 PM, Alan Bateman wrote: The approach looks good, seems like this one was lurking (for protected members at least) for a long time. Yes and this issue becomes more noticeable in JDK 9 as public members needs additional module access check. The 3 x getRoot methods on

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-30 Thread Alan Bateman
On 28/04/2018 10:44, mandy chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.00/ The reflection machinery stores the caller class in each AccessibleObject such that it can skip the access check if access to a member has been verified for a given caller.  At

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-29 Thread mandy chung
On 4/28/18 6:52 PM, Peter Levart wrote: Hi Mandy, On 04/28/18 11:44, mandy chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.00/ The reflection machinery stores the caller class in each AccessibleObject such that it can skip the access check if access to

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-28 Thread Peter Levart
Hi Mandy, On 04/28/18 11:44, mandy chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.00/ The reflection machinery stores the caller class in each AccessibleObject such that it can skip the access check if access to a member has been verified for a given

Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-28 Thread mandy chung
Webrev: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.00/ The reflection machinery stores the caller class in each AccessibleObject such that it can skip the access check if access to a member has been verified for a given caller.  At the first time accessing the