Fwd: Question about Reflection details

[Peter Levart]

Hi Thomas,

On 05/15/18 14:20, Thomas Stüfe wrote:
> Hi all,
>
> Redirecting my question to core-libs as suggested by David.
>
> Thanks, Thomas
>
>
> ---------- Forwarded message ----------
> From: Thomas Stüfe <thomas.stuefe at gmail.com>
> Date: Tue, May 15, 2018 at 12:06 PM
> Subject: Question about Reflection details
> To: Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>
>
>
> Hi all,
>
> I have two questions about the non-native reflection mechanism in the VM:
>
> 1) For each method invocation we generate a new child class of
> internal.reflect.MagicAccessorImpl, which is loaded by its own
> instance of DelegatingClassLoader.

That's not entirely true. Only the Nth invocation of a particular Method 
generates it. Unless overridden by a system property, the 1st invocation 
constructs a NativeMethodAccessorImpl and wraps it with 
DelegatingMethodAccessorImpl which delegates invocations to the former 
which uses JNI to perform the invocations and also counts them. By 
default, after 15 invocations, NativeMethodAccessorImpl replaces itself 
as the delegate of DelegatingMethodAccessorImpl with a generated 
subclass of MethodAccessorImpl which is a subclass of MagicAccessorImpl 
as you pointed out. So in reality, only the 15th invocation triggers 
generation of new class and its loading in new class loader...

> The comment in jdk.internal.reflect.ClassDefiner states the reasons for this:
>
> <quote>
> There are two primary reasons for creating a new loader
> instead of defining these bytecodes directly into the defining
> loader of the target class: first, it avoids any possible
> security risk of having these bytecodes in the same loader.
> Second, it allows the generated bytecodes to be unloaded earlier
> than would otherwise be possible, decreasing run-time
> footprint.
> </quote>
>
> Do I understand this correctly:
>
> the lifetime of the MagicAccessorImpl instance, its class and its
> loading DelegatingClassLoader are defined by the lifetime of the
> java.lang.reflect.Method/Constructor object which keeps the
> MagicAccessorImpl instance in its methodAccessor/constructorAccessor
> field?

Right, but this same MethodAccessor instance is also set on the 
corresponding "root" Method instance - this is the instance that is 
never exposed to users, but is kept in per-declaring-Class cache. When 
users ask for Method/Field/Constructor, they get a copy of the "root" 
object, because Method/Field/Constructor are unfortunately mutable 
objects (remember .setAccessible())...

So the MethodAccessor is also reachable from the "root" Method object, 
which is softly reachable from the method's declaring Class. The cache 
of Methods/Fields/Constructors in Class object is managed by a 
SoftReference. So in theory, in the presence of memory pressure, cached 
Methods/Fields/Constructors may be collected and their XxxAccessors with 
them and the generated classes too.

> So, if that Method/Constructor object is collected, its
> MagicAccessorImpl instance is collected, and since it is the only
> instance its class too, and since it is the only class in that
> DelegatingClassLoader that gets collected as well?

Right.

> 2) I see in my traces that for each Method.invoke(obj.foo()) we generate
>    - one GeneratedMethodAccessorImplXXX class living in its own
> DelegatingClassLoader instance, which invokes obj.foo

As described above, not in each invocation, but only in the 15th 
invocation and never more for the same Method (unless it gets unloaded 
in the meantime of course).

>    - and then one additional GeneratedConstructorAccessorXXX, again
> living in its very own DelegatingClassLoader instance, which invokes
> the constructor for the just generated GeneratedMethodAccessorImplXXX.

This should not happen as that "newInstance()" invocation should be the 
1st and the only invocation of the Constructor for that 
GeneratedMethodAccessorImplXX subclass.

Unless you set a system property to disable inflation in which case the 
GeneratedMethodAccessorImplXXX is generated on 1st invocation of the 
Method and constructing its instance triggers generating 
GeneratedConstructorAccessorXXX on its 1st invocation too. But only the 
1st invocation. Following invocations of the same Method just use the 
already instantiated accessor object.

> The latter I see only if I switch off inflation. The very first (and
> only) time GeneratedMethodAccessorImplXXX is instantiated it will
> cause GeneratedConstructorAccessorXXX to be created for that one
> newInstance() call.

Righ. Your observation is correct.

> But surely, in that case we could save one class loader and let the
> GeneratedConstructorAccessorXXX get loaded by the
> DelegatingClassLoader which also loaded the associated
> GeneratedMethodAccessorImplXXX? Or is it too much bother, since we are
> in an artificial scenario (noInflation=true)?

Better yet, GeneratedXXXAccessor(s) should be instantiated (constructed) 
by special constructor accessors - just like the 
GeneratedConstructorAccessorXXX(s) are - 
BootstrapConstructorAccessorImpl is used for that purpose, to prevent 
dependency loop. It would not be hard to re-purpose it to instantiate 
all Generated[Field,Method,Constructor]AccessorXXX subclasse(s) 
directly, using Unsafe.allocateInstance().

> ----
>
> In general, I have the following question:
>
> Will the 1:1 relationship between MagicAccessorImpl class and
> DelegatingClassLoader instance always hold true?
>
> Or is there work in progress somewhere which would maybe bundle
> MagicAccessorImpl classes in one loader (e.g. (2) would be a candidate
> for this), or maybe do it without loaders?

GeneratedMethodAccessor(s) as subclasses of MagicAccessorImpl are 
treated specially by JVM verifier. Their bytecode is free to invoke any 
method. Method.invoke API performs access checks on behalf of the 
Method.invoke caller programatically, but the bytecode that does the 
actual invocation lives in the GeneratedMethodAccessorXXX and that's why 
it must be allowed to invoke any method. Having access to a Class object 
representing a subclass of MagicAccessorImpl is a security risk since 
instantiating such class gives the caller an object which is able to 
invoke the method whichever that is (without the programmatic shield of 
the Method object). Loading it in a special ClassLoader is therefore a 
measure against exploiting such generated class as such class loader is 
not in the delegating chain of the caller. and so caller can not resolve 
it by name. Multiple GeneratedMethodAccessorXXX(s) for the method of the 
same declaring class could be loaded by the same class loader though, 
because that would usually be the granularity of clearing the softly 
reachable cache of Methods in the Class object(s). But such optimization 
has not been attempted yet, I suppose.

Another trick would be to use anonymous VM class-es to materialize the 
code of GeneratedMethodAccessor(s). They could be defined by any 
existing class loader and they would:
- not present a security risk as they are not resolvable by name
- be able to be unloaded before the defining class loader as they are 
not retained by it


> Thanks!
>
> Thomas

Thanks for presenting the opportunity for optimization. Will you be 
willing to try implementing it?

Regards, Peter