[Nestmates] RFR (S): Dynamic nestmate update

[David Holmes]

Hi Karen,

Thanks for taking a look.

On 31/10/2018 12:00 AM, Karen Kinnear wrote:
> Agree with setting the dynamic_nest_host before publishing.
> Agree with disallowing mixing static and dynamic membership.
> 
> JVMTI - not clear on this one. The CFLH generally does not follow the JVMTI restrictions, since it
> is viewed as completely replacing the class file before it is ever seen, so there are currently (AFAICT)
> no restrictions on what can be done at this time.
> 
> I don’t know the best way to handle that. One approach would be to just throw the current ClassFormatError
> and agent authors will learn that they can not define NestHost or NestMembers even at CFLH time.

Does CFLH currently transform unsafe anonymous classes? I can see both 
pluses and minuses for allowing that. On the plus side simple 
instrumentation applies to all classes, but dynamically generated 
classes only have to follow VM rules, not language rules, so an agent 
could easily try to do something that is not appropriate or applicable 
to a dynamically generated classfile. So this seems to be a "thin ice" 
situation - if it works great, if it doesn't the agent authors can't 
really complain too much.

Dealing with NestHost and NestMembers is new territory for agents. The 
"best practice" there is that these attributes should simply be 
preserved, unless explicitly trying to modify something related to nests 
(not permitted for redefinition/retransformation). If they are trying to 
modify something related to nests then they will have to be informed 
about dynamic nest membership somehow. Not sure how - perhaps the origin 
of the initial bytes ie Classloader.defineClass versus Lookup.defineClass?

> They certainly have no way to distinguish dynamically defined classes from statically defined classes.
> Perhaps it would make sense to add the the JVMTI specification that this is true even at CFLH time?
> If so, we would need to perform a before/after check.

I would not want to add something that requires an expensive validity 
check at runtime. What we did for retransform/redefinition was bad 
enough. :(

Cheers,
David

> 
> Thoughts?
> 
> thanks,
> Karen
> 
> 
>> On Oct 30, 2018, at 12:17 AM, Mandy Chung <mandy.chung at oracle.com> wrote:
>>
>>
>>
>> On 10/29/18 4:35 PM, David Holmes wrote:
>>>
>>>> - At the time class file nest members attributes are parsed and the nest host setting within the byte stream matches the nest host used when jvm_lookup_define_class() is invoked, than in my opinion an error should not result.
>>>
>>> I disagree. You're either using dynamic nest membership or you're using static nest membership - using both, even if the end result would be the same, seems an error in the programming model to me. Afterall if the defined class is already a valid static nest member then you don't need to use the NESTMATE property to inject it. And it is definitely an error to claim a static nest-host when the host does not statically list you as a member!
>>
>> I also considered whether it should be accepted or an error if the NestHost attribute exists and matches the nest host class being injected it.   I agree with David to separate the static nest membership model from dynamic nest membership.  We can revisit that when there is a good use case to accept NestHost attribute matching the nest host in the future.
>>>
>>>> - Finally, should the setting of nest_host be known at the point class file load hook is processed?
>>>
>>> I'm not sure what you mean. At what point in the current code would the byte[] passed to defineClass be replaced by one from the CLFH? As long as the byte[] is final before we actually set_nest_host, I don't think it matters.
>>>
>>
>> JVM TI currently restricts that the retransformation must not change the NestHost or NestMembers attributes.  If the class bytes are transformed, it will remain as a dynamic nestmate and no NestHost and NestMembers attributes.  It seems that the nest host does not need to be known by CFLH.
>>
>> Mandy
>