Jigsaw Enhancement RFR round #3: 8159145 Add JVMTI function GetNamedModule
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Thu Jun 30 00:21:46 UTC 2016
Hi Dan,
On 6/29/16 07:59, Daniel D. Daugherty wrote:
> On 6/29/16 2:53 AM, serguei.spitsyn at oracle.com wrote:
>> On 6/29/16 01:45, Jesper Wilhelmsson wrote:
>>> 29 juni 2016 kl. 04:30 skrev David Holmes <david.holmes at oracle.com>:
>>>>> On 29/06/2016 12:09 PM, serguei.spitsyn at oracle.com wrote:
>>>>>> On 6/28/16 18:44, David Holmes wrote:
>>>>>>> On 29/06/2016 7:09 AM, serguei.spitsyn at oracle.com wrote:
>>>>>>>> On 6/28/16 14:02, Daniel D. Daugherty wrote:
>>>>>>>>> On 6/28/16 2:11 PM, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>> On 6/28/16 11:19, Daniel D. Daugherty wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I'll have to check the upper layers of this API, but
>>>>>>>>>> if an
>>>>>>>>>> agent can pass a bad 'class_loader' parameter and get
>>>>>>>>>> this
>>>>>>>>>> assert() to fire, then that's not good. Hopefully a bad
>>>>>>>>>> 'class_loader' parameter is caught at a higher layer.
>>>>>>>>> Not sure, what do you mean.
>>>>>>>>> Do you mean the generated JVMTI upper layer or the
>>>>>>>>> JvmtiEnv::GetNamedModule?
>>>>>>>>> Probably, the generated code.
>>>>>>>> I did mean the generated layer.
>>>>>>> Ok, thanks.
>>>>>>>
>>>>>>>>
>>>>>>>>>> Update: Yes, passing a non-NULL jobject as the
>>>>>>>>>> class_loader
>>>>>>>>>> parameter
>>>>>>>>>> when the jobject does not refer to a "class
>>>>>>>>>> loader" is
>>>>>>>>>> caught
>>>>>>>>>> at the upper layer.
>>>>>>>>> The upper layer does not check that it is a class loader, just
>>>>>>>>> for
>>>>>>>>> non-NULL.
>>>>>>>>> I think, it is good to have an assert here to double-checks the
>>>>>>>>> pre-conditions as other caller can be added later.
>>>>>>>>> But I'm Ok to get rid of it if you suggest.
>>>>>>>> Please keep the asserts. Basically I was mumbling to myself to
>>>>>>>> make sure that the asserts could not be reached by user code
>>>>>>>> and the "Update:" was to indicate that I did do.
>>>>>>> Ok, thanks.
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>>> src/share/vm/prims/jvmti.xml
>>>>>>>>>> L6550: <param id="module_ptr">
>>>>>>>>>> L6551: <outptr><jobject/></outptr>
>>>>>>>>>> L6552: <description>
>>>>>>>>>> L6553: On return, points to a
>>>>>>>>>> <code>java.lang.reflect.Module</code> object.
>>>>>>>>>> L6554: </description>
>>>>>>>>>> L6555: </param>
>>>>>>>>>>
>>>>>>>>>> The above wording doesn't allow for module_ptr to be
>>>>>>>>>> NULL
>>>>>>>>>> which
>>>>>>>>>> is a mismatch with the description.
>>>>>>>>> I disagree (or maybe I got it incorrectly).
>>>>>>>>> Pointing to NULL and be NULL is different.
>>>>>>>>> It is not allowed for the module_ptr to be NULL but Ok to pint to
>>>>>>>>> NULL on return.
>>>>>>>> I think the description needs to be:
>>>>>>>>
>>>>>>>> On return, points to a
>>>>>>>> <code>java.lang.reflect.Module</code> object
>>>>>>>> or points to a <code>NULL</code>.
>>>>>>> Agreed, fixed.
>>>>>> Disagree. You would say that a pointer is NULL, not that it
>>>>>> points to
>>>>>> a NULL.
>>>>> Why are you disagree?
>>>>> The module_ptr is an out argument, it is not allowed to be NULL.
>>>>> But the returning value by this pointer can be NULL.
>>>> As per later email I see this terminology already exists so is
>>>> fine, but I find it confusing to say "points to a NULL" - a NULL is
>>>> not an entity. If "foo points to a NULL" that implies to me "foo ==
>>>> &NULL;" which is nonsense - foo == NULL, and if foo==NULL then foo
>>>> points to nothing. But the "pointer to a pointer" aspect here does
>>>> confuse things.
>>> I would prefer if it said "points to a NULL pointer", or
>>> NULL-pointer. I'm not sure what would be the more correct way to
>>> write it. Anyway, a NULL pointer is an entity :-)
>> Jesper,
>>
>> Thank you for the comment.
>> In fact, I've just used a pattern that is already present in the
>> JVMTI spec.
>> Objections to the existing pattern are minor, so it is better to be
>> more conservative here.
>
> Perhaps we can use the wording in this JVM/TI function as a model:
>
> http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#GetCurrentContendedMonitor
>
>
> This function takes a "jobject *" as a parameter and returns a
> monitor pointer via that parameter. Pretty similar to what we're
> discussing...
>
> So here's the existing wording example:
>
> Name Type Description
> =========== ======== ===========
> monitor_ptr jobject* On return, filled with the current contended
> monitor,
> or NULL if there is none.
>
> Agent passes a pointer to a jobject. On return,
> the
> jobject has been set. The object returned by
> monitor_ptr
> is a JNI local reference and must be managed.
>
> So for the new function:
>
> module_ptr jobject* On return, filled with a
> <code>java.lang.reflect.Module</code>
> object or NULL if there is none.
>
> This "filled with" style is used for return parameters in
> ~13 places in the JVM/TI spec.
Thank you for pointing to the example.
This discussion deviated to the question what the JVMTI pattern is
better to copy as a best practice.
>
> Of course, now that I've found the GetCurrentContendedMonitor
> example, That second paragraph or something like it is also
> going to be needed with our new function...
Just wanted to note that this paragraph is not strictly necessary as
it is already covered in the common part of the spec:
http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#refs
However, the second sentence is auto-generated from the jvmti.xml:
"Agent passes a pointer to a |jobject|. On return, the |jobject| has
been set.
The object returned by |module_ptr| is a JNI local reference and must be
managed."
Please, find the specdiff in the attachment.
Thanks,
Serguei
>
> Spec wording is very difficult... :-)
>
> Dan
>
>
>>
>> Thanks,
>> Serguei
>>
>>
>>
>>> /Jesper
>>>
>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>>> David
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Dan
>>
>
More information about the hotspot-dev
mailing list