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

[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 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.
>

I started with that approach but I wanted to assert that the root object 
is used to catch any future regression of the memory leak. There is 
other option doing that for example adding LangReflectAccess::isRoot or 
ensureRoot instead of getRoot method. I see all these are part of the 
reflection implementation.  We could revisit this code when we touch 
this area in the future.

> 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.

Sadly, there are existing code using this hack.   As Alan said, we will 
find out more once the illegal access is denied by default.  I prefer to 
separate this breakage from this issue and disable the hack to change 
static final field via reflection completely in another day.

Mandy