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
