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

[Yumin Qi]

More, the defined signals from SIGINT:

(linux):
#define BREAK_SIGNAL     SIGQUIT           /* Thread dumping support.    */
#define INTERRUPT_SIGNAL SIGUSR1           /* Interruptible I/O support. */
#define SHUTDOWN1_SIGNAL SIGHUP            /* Shutdown Hooks support.    */
#define SHUTDOWN2_SIGNAL SIGINT
#define SHUTDOWN3_SIGNAL SIGTERM

So the check signal comes from SHUTDOWN2:

  if (!ReduceSignalUsage) {
    DO_SIGNAL_CHECK(SHUTDOWN1_SIGNAL);
    DO_SIGNAL_CHECK(SHUTDOWN2_SIGNAL);
    DO_SIGNAL_CHECK(SHUTDOWN3_SIGNAL);
    DO_SIGNAL_CHECK(BREAK_SIGNAL);
  }

If using -Xrs we would not see the ouput warning.

Thanks
Yumin


On 3/14/2014 9:57 AM, Yumin Qi wrote:
> 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
>>>
>>
>