Jigsaw Enhancement RFR round #3: 8159145 Add JVMTI function GetNamedModule
Daniel D. Daugherty
daniel.daugherty at oracle.com
Thu Jun 30 02:07:03 UTC 2016
On 6/29/16 6:21 PM, serguei.spitsyn at oracle.com wrote:
> 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.
The specdiff looks good.
There is one strange sentence:
> If class_loader is NULL, the bootstrap loader.
Perhaps "the bootstrap loader is assumed."?
Your call.
Dan
>
> 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