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

[Dmitry Samersoff]

Yumin.

Yes. my bad.

-Dmitry


On 2014-03-14 21:09, Yumin Qi wrote:
> 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
>>>>
>>>
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.