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

[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 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 incompatibility from this 
> issue.  I modified the fix to change the modifiers of the root and 
> child field object be the same.
>
> http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.02/
>
> Mandy

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 cleaner to leave the 
ReflectionFactory as is, but modify the following private methods instead:

- Field#acquireFieldAccessor
- Method#acquireMethodAccessor
- Constructor#acquireConstructorAccessor

They already deal with 'root' object and could pass it to 
ReflectionFactory. The logic of ReflectionFactory need not deal with the 
notion of 'root' object then and no LangReflectAccess additions are 
necessary. You would keep the notion of root objects encapsulated. With 
tour patch it has leaked into ReflectionFactory too.

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 never reload such field's value.

Regards, Peter