[Nestmates] Add a core reflection API to get nestmate information

[Peter Levart]

Hi,

I saw the proposal for a core reflection API to get nestmate information 
and here are my thoughts about the decision to filter out class 
resolution errors in some methods, discussed below...

On 11/08/2017 01:19 AM, David Holmes wrote:
> Hi Dan,
>
> Thanks for taking a look at this.
>
> On 8/11/2017 9:21 AM, Dan Smith wrote:
>> I would be careful to match the behavior of 'isNestmateOf' to JVMS 
>> 5.4.4, rather than define it in terms of 'getNestHost'. In 
>> particular, 'c.isNestmateOf(c)' shouldn't need to perform any class 
>> loading.
>
> I could short-circuit that case, but why special case this instead of 
> just retrieving the nest host (which may need to be loaded)? This is 
> not an access-check (for which we bail out very early for the same 
> class) but a simple query, so the access-check process per JVMS 5.4.4 
> don't directly need to apply.
>
>> For other cases, if class loading errors occur, is the proposal to 
>> swallow them and return 'false'? That seems okay, I guess, but is a 
>> different conclusion than what we came up with in JVMS.
>
> This was recently discussed on this list under "nestmates spec open 
> issues".
>
> http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2017-October/000386.html 
>
>
> John prefers to minimize exceptions for the Java API. :)

Suppose that this is basic reflection API to get nestmate information 
and that it will also be used for reflective access checks. For example 
in jdk.internal.reflect.Reflection#verifyMemberAccess, which is used 
from Method.invoke, Fileld.[get|set], Constructor.newInstance. The most 
appropriate method for this check is Class.isNestmateOf(Class). For example:

     /**
      * Verify access to a member and return {@code true} if it is granted.
      *
      * @param currentClass the class performing the access
      * @param memberClass the declaring class of the member being accessed
      * @param targetClass the class of target object if accessing instance
      *                    field or method;
      *                    or the declaring class if accessing constructor;
      *                    or null if accessing static field or method
      * @param modifiers the member's access modifiers
      * @return {@code true} if access to member is granted
      */
     public static boolean verifyMemberAccess(Class<?> currentClass,
                                              Class<?> memberClass,
                                              Class<?> targetClass,
                                              int modifiers)
     {
         if (currentClass.isNestmateOf(memberClass)) {
             // Always succeeds
             return true;
         }
         ...

If there is a nest-host class resolution error and isNestmateOf() 
returns false for two actual nest-mates that have lost their host, 
reflective access is going to throw IllegalAccessException(s) and the 
real cause of failure will be hidden. That doesn't help to diagnose the 
problem.

If the reasoning behind the decision to hide resolution exceptions is 
the desire to mimic OLD behavior, it is not successful, because the OLD 
behavior allows private access between two nested classes when "top" 
class is missing while then NEW behavior disallows it.

Enter symmetric scheme: instead of one nest-host with a list of 
nets-matest + N-1 nest-mates that reference just the nest-host, there 
would be N nest-mates with equal status where each lists all others. 
With symmetric scheme there would be no need to hide resolution errors 
that happen now when access checks are performed between two nest-mates 
where neither of them is a nest-host.

If I remember correctly, the symmetric scheme was not considered, 
because it would be complicated to resolve inconsistencies and spoofing.

Is it really that important to have a notion of nest-host? Is it really 
that important to be able to (consistently) enumerate members of nest? 
What problem are nest-mates solving? If the answer is just "access 
checking", then the notion of nest-host and nest-mates enumeration are 
only artificial implementation details. If those two concepts are really 
not necessary ingredients, then a symmetric scheme is a simple 
alternative for access checking. The only method needed is this:

Class.isNestmateOf(Class<?> c);

A method that returns true if (this == c || <this and c have each other 
in their lists of nest-mates>).

Spoofing is not possible, because both parties must agree and resolution 
errors don't need to be hidden, because only resolution of involved 
parties is necessary.

You still have reflexive and symmetric, but you loose transitive 
property: if A may access B.private and B may access C.private that 
doesn't imply that A may access C.private, because the nest-lists of 
classes may be:

A: [B]
B: [A, C]
C: [B]

But such cases may only arise as a consequence of faulty assembly. 
Compilation (even partial/incremental) can always verify the consistency 
of the whole nest and bail-out if needed.

If you really needed to enumerate the members of the nest, the result 
would be dependent on the member that was given as an anchor point. A 
good-enough algorithm is this:

x.getNestMembers() returns a list composed of x and all classes listed 
in x's nest-list that also have x listed in their nest-list(s). For 
above inconsistent example that would mean:

A.class.getNestMembers() = [A, B];
B.class.getNestMembers() = [A, B, C];
C.class.getNestMembers() = [B, C];

But then again, if you are asking a trusted class for the list of nest 
members, no-one can spoof and add additional members to the list that 
was established when that trusted class was compiled.

To wrap-up: symmetric scheme has pros and cons. The question is what is 
more important - the consistent view (no matter who you ask, you always 
get the same answer) but with potential resolution troubles when 
nest-host is not resolvable, or a semi-consistent view (depends on who 
you ask, but if you ask a trusted class, you get a trusted answer) but 
never need for resolving any additional classes.

Regards, Peter