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

Fri Mar 14 18:35:19 UTC 2014

Karen and all,

   http://cr.openjdk.java.net/~minqi/6536943/webrev03/

   This version only still keeps original version of checking for signal 
plus more info printout like:

Warning: SIGINT handler expected:libjvm.so+0x894d30 found:0x0000000000000001
Running in non-interactive shell, SIGINT handler is replaced by shell   
<--------------------------- newly added output
Signal Handlers:
SIGSEGV: [libjvm.so+0x892040], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGBUS: [libjvm.so+0x892040], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGFPE: [libjvm.so+0x892040], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGPIPE: [libjvm.so+0x892040], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGXFSZ: [libjvm.so+0x892040], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGILL: [libjvm.so+0x892040], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGUSR1: SIG_DFL, sa_mask[0]=00000000000000000000000000000000, sa_flags=none
SIGUSR2: [libjvm.so+0x893710], 
sa_mask[0]=00000000000000000000000000000000, sa_flags=SA_RESTART|SA_SIGINFO
SIGHUP: [libjvm.so+0x894d30], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGINT: SIG_IGN, sa_mask[0]=00000000000000000000000000000000, 
sa_flags=none <------------------------------------------------- user 
can get hint from here
SIGTERM: [libjvm.so+0x894d30], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
SIGQUIT: [libjvm.so+0x894d30], 
sa_mask[0]=11111111011111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO

Let me know if it is OK.

Thanks
Yumin

On 3/14/2014 10:22 AM, Karen Kinnear wrote:
> Yumin,
>
> I had the same question.
>
> Another possibility would be to leave the check in, and to add a comment
> after reporting SIGINT as SIG_IGN that this could be an artifact of running in a non-interactive shell
>
> opinions?
>
> thanks,
> Karen
>
> On Mar 14, 2014, at 1:17 PM, Yumin Qi wrote:
>
>> Karen,
>>
>>   I agree only report non-interactive shell warning --- if under other case user replace the signal handler, we should give warning. This lets me think of the fix one more time, do we really want to silent for non-reactive shell? It is no harm for customer knows what happened --- it is the shell replace the handler. If user understand that, it is OK I think.
>>
>> Thanks
>> Yumin
>>
>> On 3/14/2014 10:10 AM, Karen Kinnear wrote:
>>> #define SHUTDOWN_SIGNAL1 SIGINT
>>>
>>> I also wondered about checking if the current SIGINT interrupt handler setting was
>>> SIG_IGN.
>>>
>>> The question is - do we want to only not report for non-interactive or for anyone who
>>> has set SIG_IGN? I was leaning in the direction of non-interactive.
>>>
>>> thanks
>>> Karen
>>>
>>> On Mar 14, 2014, at 12:57 PM, 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