Review Request: JDK-8206240: java.lang.Class.newInstance() is causing caller to leak
peter.levart at gmail.com
Wed Oct 3 16:30:46 UTC 2018
I don't know if this matters though, but should
Reflection.getCallerClass() ever return null, previous behavior was to
throw NPE (from Reflection.verifyMemberAccess(...)), now the checks are
skipped. This should only be observable if [Class,
Constructor].newInstance() was called from JNI's newly attached thread I
believe. You know the inner workings of Reflection.getCallerClass() more
than I do, so is it trust-able to always return non-null?
On 10/03/2018 05:14 PM, Mandy Chung wrote:
> On 10/3/18 6:02 AM, Alan Bateman wrote:
>> On 02/10/2018 20:33, Mandy Chung wrote:
>>> Class::newInstance maintains its separate cache of the caller
>>> class after access check. This leak has been there for a long
>>> time and unnoticed.
>>> This patch changes Class::newInstance to use the code path
>>> for Constructor::newInstance doing the access check and
>>> caching the caller class. It will also get the illegal
>>> access check in effect when Class::newInstance is called
>>> consistent with Constructor::newInstance.
>>> Webrev at:
>>> Alan raises a question whether the cache is still needed
>>> given that we have improved the performance of slow
>>> path of access check with Class::getPackageName. It's a
>>> good investigation to follow up.
>> The changes look okay. One niggle is Cache.callerClassCache as
>> nothing to do with Cache, maybe the method should be removed and the
>> one usage changed to create the weak ref.
> Thanks. I have removed the callerClassCache method as you suggest.
>> On the benefit of the cache then I think it would be good to get some
>> data on the benefit. Prior to the JDK 9 then I think most of the cost
>> was checking the package but that has mostly disappeared with the
>> changes in 9.
> I created https://bugs.openjdk.java.net/browse/JDK-8211452 as a follow
More information about the core-libs-dev