Review Request: JDK-8206240: java.lang.Class.newInstance() is causing caller to leak

[Mandy Chung]

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:
>> http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8206240/webrev.00
>>
>> 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 up.

Mandy