RFR(XS) 8212612 - Add documentation about Arguments::_exit_hook

[David Holmes]

+1 on the updated comment.

Thanks,
David

On 18/10/2018 9:59 AM, dean.long at oracle.com wrote:
> On 10/17/18 3:44 PM, Ioi Lam wrote:
>>
>>
>>
>> On 10/17/2018 02:59 PM, dean.long at oracle.com wrote:
>>> On 10/17/18 2:35 PM, Ioi Lam wrote:
>>>>
>>>>
>>>> On 10/17/2018 02:27 PM, dean.long at oracle.com wrote:
>>>>> On 10/17/18 9:33 AM, Ioi Lam wrote:
>>>>>> http://cr.openjdk.java.net/~iklam/jdk12/8212612-exit-hook-docs.v01/
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8212612
>>>>>>
>>>>>> The following fields are assigned to according to 
>>>>>> JavaVMOption.optionString.
>>>>>>
>>>>>>   abort_hook_t Arguments::_abort_hook = NULL;
>>>>>>   exit_hook_t Arguments::_exit_hook = NULL;
>>>>>>   vfprintf_hook_t Arguments::_vfprintf_hook = NULL;
>>>>>>
>>>>>> However, there's no code in the JDK repo that uses these options.
>>>>>> Instead, they are intended for programs that embed the JVM using
>>>>>> JNI_CreateJavaVM.
>>>>>>
>>>>>> We should add comments in arguments.cpp for clarification. Otherwise
>>>>>> this would look like dead code and someone might be tempted to
>>>>>> remove it.
>>>>>>
>>>>>> Thanks
>>>>>> - Ioi
>>>>>
>>>>> But it's not dead code.  All three are used.  For example, see 
>>>>> jio_vfprintf().
>>>>>
>>>>> dl
>>>>
>>>> Hi Dean, thanks for looking into this.
>>>>
>>>> I was afraid someone would infer that since nowhere in the JDK code 
>>>> would do anything to assign _vfprintf_hook to a non-NULL value, the 
>>>> block
>>>>
>>>>   if (Arguments::vfprintf_hook() != NULL) {
>>>>     jio_fprintf(defaultStream::output_stream(), "%.*s", (int)len, s);
>>>>   }
>>>>
>>>> would be effectively dead code.
>>>>
>>>
>>> OK, then this comment was confusing:
>>>
>>> These are not used by the JDK itself
>>>
>>> I think it's more accurate to say that JDK launchers don't set these, 
>>> but the JVM does use and honor them if set.
>>>
>>
>> How about this:
>>
>> // These are not set by the JDK's built-in launchers, but they can be 
>> set by programs that
>> // embed the JVM using JNI_CreateJavaVM. See comments around 
>> JavaVMOption in jni.h.
>> abort_hook_t     Arguments::_abort_hook         = NULL;
>> exit_hook_t      Arguments::_exit_hook          = NULL;
>> vfprintf_hook_t  Arguments::_vfprintf_hook      = NULL;
>>
>>
> 
> That's good!
> 
> dl
> 
>> Thanks
>> - Ioi
>>
>