Hi Mandy,
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?
Regards, Peter
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:
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
