help with changes to hotspot runtime to implement sealed types

[Vicente Romero]

On 12/11/18 9:36 PM, David Holmes wrote:
> On 12/12/2018 12:32 pm, Vicente Romero wrote:
>> On 12/11/18 9:21 PM, David Holmes wrote:
>>> Hi Vincente,
>>>
>>> On 12/12/2018 11:53 am, Vicente Romero wrote:
>>>> Hi HS runtime,
>>>>
>>>> I'm working on the implementation of sealed types one of the 
>>>> projects under the Amber umbrella. A sealed type is one that can 
>>>> only be subtyped by a set of classes declared by the sealed type in 
>>>> Java:
>>>>
>>>> final class Sealed permits Sub {}
>>>>     class Sub extends Sealed {}
>>>
>>> This has evolved :) I was expecting it to be restricted to 
>>> subclassing by nestmates.
>>
>> :) yep
>>
>>>
>>>> Both Sealed and Sub can coexist in the same compilation unit, 
>>>> package or module. The class file for Sealed have an attribute 
>>>> named PermittedSubtypes that contains a list with the, well, 
>>>> permitted subtypes. In the case of the example it will be Sub only. 
>>>> On top of that class Sealed will be final. So when loading / 
>>>> parsing class file for Sub, we need to check, given that its 
>>>> supertype is final, if Sub is listed as one of the permitted 
>>>> subtypes of Sealed or fail in other case. And here is when I'm 
>>>> finding a roadblock. Currently the VM doesn't allow any class to 
>>>> subtype a final class and this is checked at:
>>>>
>>>> ClassFileParser::post_process_parsed_stream
>>>>
>>>> so I removed the code that was doing this check at 
>>>> `ClassFileParser` and created a separate method to do the checking 
>>>> at InstanceKlass with the idea of enforcing the check after the 
>>>> class file was parsed and the given class loaded. But regardless of 
>>>> my intents I always get a SOE. The SOE happens when the subclass 
>>>> ask the super class if the subclass is listed as one of its 
>>>> permitted subtypes. To answer this, the superclass tries to load 
>>>> the subclass so I have a deadlock that I suspect could be hard to 
>>>> solve, if possible. Please see attached what I have done so far. 
>>>> Any ideas?
>>>
>>> +      if (name == k->name()) {
>>> +        log_trace(class, sealed)("- Found it at 
>>> permitted_subtypes[%d] => cp[%d]", i, cp_index);
>>> +        // names match so check actual klass - this may trigger 
>>> class loading if
>>> +        // it doesn't match (but that should be impossible)
>>> +        Klass* k2 = _constants->klass_at(cp_index, CHECK_false);
>>>
>>> You can't check the actual class in your case as if the names match 
>>> then it refers to the subclass you are currently trying to load.
>>
>> and what should I do? just check by name without doing:
>>
>> Klass* k2 = _constants->klass_at(cp_index, CHECK_false);
>
> Yes. All you can check is that the name of the current class matches 
> an allowed subtype, and that the current class will be loaded into the 
> same classloader as the supertype, and hence in the same package and 
> module.
>
> Note that the nestmates code you are copying deals with already loaded 
> classes. Nest membership is not checked at classload time, only when a 
> nestmate access check is needed. Whereas your code is dealing with a 
> currently-being-loaded class, so needs to be more careful.

I see, yes I realized that the related nestmates code was invoked by the 
JNI but I thought that the check could not be complete, or strong 
enough, if I didn't compare with the actual class. But what you are 
saying solves my problem :)

>
> Cheers,
> David

Thanks!
Vicente

>
>>
>>>
>>> HTH,
>>> David
>>
>> Thanks,
>> Vicente
>>>
>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> PD, the code runs with no error with the slowdebug configuration
>>