jdk.internal.reflect.ReflectionFactory and SecurityManager
david.holmes at oracle.com
Tue Dec 27 01:02:54 UTC 2016
On 27/12/2016 9:48 AM, Claes Redestad wrote:
> while perhaps an enhancement in isolation, I'll argue this to be a
> blocker of or a sub-task of the redo of JDK-8062389 - a P2 bug that
At this stage I would categorise it as one possible way to work around
the issue that JDK-8062389 et al. ran into.
The initialization sequence is fragile and is not a constant - different
runtime options can affect the paths taken. So testing anything that
introduces a change to initialization order is tricky, as there is no
obvious set of tests that provide full coverage.
> has been long in the making - thus I don't agree that the time for this
> has passed, and neither do I think this needs a critical approval
> request if it's pushed to enable a redo of JDK-8062389 et al.
> On 2016-12-26 22:16, David Holmes wrote:
>> Hi Peter,
>> I know this is response to the problems with your other recent change,
>> but this is an enhancement in my opinion, not a bug fix, and the time
>> for enhancements is passed - though there is still a process to request
>> them. I do not like to see last minutes changes like this where not
>> enough due diligence may be done to identify all the ramifications.
>> The Class.getMethod() fixes seem to have overstepped the line in that
>> regard as well, in my opinion. Anything that potentially changes
>> initialization order is fragile and risky and requires additional
>> That said, I am an admirer of your work on OpenJDK! :)
>> On 27/12/2016 5:29 AM, Peter Levart wrote:
>>> There are 2 ReflectionFactory classes in JDK 9. The old one is
>>> sun.reflect.ReflectionFactory which ended in jdk.unsupported module and
>>> to which access is restricted with SecurityManager. There is also new
>>> jdk.internal.reflect.ReflectionFactory which is used internally by JDK,
>>> is exported to internal modules only but still uses SecurityManager to
>>> restrict access to itself. I checked all usages and they all use
>>> AccessControler.doPrivileged() for obtaining the instance of
>>> jdk.internal.reflect.ReflectionFactory, which somehow defeats the
>>> purpose of SecurityManager access checks in this API.
>>> I think this could be simplified by removing the SecurityManager check
>>> from jdk.internal.reflect.ReflectionFactory#getReflectionFactory static
>>> method and change all usages to invoke this method directly without
>>> doPrivileged(). There are already two sensitive internal APIs exposed
>>> without SecurityManager checks: jdk.internal.misc.Unsafe#getUnsafe and
>>> various jdk.internal.misc.SharedSecrets#getXxxAccess methods. So why
>>> wouldn't internal ReflectionFactory be exposed the same way?
>>> This would make obtaining the ReflectionFactory more robust and not
>>> sensitive to bootstrap issues that surfaced recently after my push of a
>>> fix for issues 8062389, 8029459, 8061950.
>>> So, what do you think? Is this a worthwhile cleanup and simplification?
>>> Regards, Peter
More information about the core-libs-dev