RFR: 6536943: Bogus -Xcheck:jni warning for SIG_INT action for SIGINT in JVM started from non-interactive shell

[Yumin Qi]

Dmitry,

   #define INTERRUPT_SIGNAL SIGUSR1

    I think this is not SIGINT.

Thanks
Yumin

On 3/14/2014 9:27 AM, Yumin Qi wrote:
> Dmitry,
>
>     case INTERRUPT_SIGNAL:
>     jvmHandler = CAST_FROM_FN_PTR(address, SIG_DFL);
>     break;
>
> Do you mean we use
>     if (isatty(fileno(stdin)))
>        jvmHandler = CAST_FROM_FN_PTR(address, SIG_DFL);
>     else
>        jvmHandler = CAST_FROM_FN_PTR(address, SIG_IGN);
>
> I think this will solve the problem. Need more opinions.
>
> Under redirecting,  isatty still can correctly tests for 
> non-interactive shell, please check bug comments.
>
> Thanks
> Yumin
>
>
> On 3/14/2014 9:05 AM, Dmitry Samersoff wrote:
>> Yumin.
>>
>> I find the only place per os_*.cpp where we check for SIGINT
>>
>> os/linux/vm/os_linux.cpp
>>
>> 4498:  DO_SIGNAL_CHECK(INTERRUPT_SIGNAL);
>>
>> so it might make sense to place it under
>>
>>    if isatty() {}
>>
>> rather than change macro.
>>
>> Also isatty() call is expensive and will not give you expected result if
>> stdin is redirected (e.g. by custom launcher)
>>
>>
>> I think it's safe to check whether the handler for SIGINT is SIG_IGN
>> and skip the check in this case.
>>
>> -Dmitry
>>
>>
>> On 2014-03-14 08:18, Yumin Qi wrote:
>>> Have changed, new URL at:
>>> http://cr.openjdk.java.net/~minqi/6536943/webrev02
>>>
>>> Thanks
>>> Yumin
>>>
>>> On 3/13/2014 4:10 PM, Yumin Qi wrote:
>>>> Thanks for review. I will change the order.
>>>>
>>>> Yumin
>>>>
>>>> On 3/13/2014 3:28 PM, Dean Long wrote:
>>>>> (isatty(fileno(stdin)) || sig != SIGINT)
>>>>>
>>>>> You might want to reverse that:
>>>>>
>>>>> (sig != SIGINT || isatty(fileno(stdin)))
>>>>>
>>>>> because isatty() could be expensive.
>>>>>
>>>>> dl
>>>>>
>>>>>
>>>>> On 3/13/2014 2:27 PM, Yumin Qi wrote:
>>>>>> David and all,
>>>>>>
>>>>>>    I have changed to skip SIGINT only if run in a non-interactive
>>>>>> shell. Please check webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~minqi/6536943/webrev01/
>>>>>>
>>>>>> Thanks
>>>>>> Yumin
>>>>>>
>>>>>> On 3/13/2014 8:19 AM, Yumin Qi wrote:
>>>>>>> On 3/12/2014 10:08 PM, David Holmes wrote:
>>>>>>>> Hi Yumin,
>>>>>>>>
>>>>>>>> Not sure that disabling all signal checks because there is a
>>>>>>>> problem with one signal is the right solution for this.
>>>>>>>>
>>>>>>> Or we check if the signal is SIGINT --- skip it only.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Yumin
>>>>>>>> That said I don't know what utility the signal checks provide in
>>>>>>>> the first place. Can anyone comment on that?
>>>>>>>>
>>>>>>>> David
>>>>>>>>
>>>>>>>> On 13/03/2014 9:55 AM, Yumin Qi wrote:
>>>>>>>>> Forget to mention tests:
>>>>>>>>> JPRT test;
>>>>>>>>> manually testing on Java2Demo, for both shell and non-interactive
>>>>>>>>> shell.
>>>>>>>>>
>>>>>>>>> This is only disabling checking signal handlers under
>>>>>>>>> non-interactive
>>>>>>>>> shell, so no need to test more.
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Yumin
>>>>>>>>>
>>>>>>>>> On 3/12/2014 2:52 PM, Yumin Qi wrote:
>>>>>>>>>> Hi, Can I have your codereview for this simple change:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~minqi/6536943/webrev00/
>>>>>>>>>>
>>>>>>>>>> Summary: Under non-interactive shell, -Xcheck:jni output
>>>>>>>>>> warning for
>>>>>>>>>> SIGINT, since non-interactive shells set SIGINT to SIG_IGN as
>>>>>>>>>> part of
>>>>>>>>>> the job control features. The fix is that don't check signal
>>>>>>>>>> handlers
>>>>>>>>>> if java run in a non-interactive shell. This applies to all
>>>>>>>>>> platforms
>>>>>>>>>> except for Windows.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Yumin
>>
>