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

Karen Kinnear karen.kinnear at oracle.com
Fri Mar 14 19:56:24 UTC 2014 

Yumin,

Thank you so much - I like this one better.

Can you please fix two things in the comment lines.
non-reactive is non-interactive
SIGIGN is SIG_IGN

thanks,
Karen

On Mar 14, 2014, at 2:35 PM, Yumin Qi wrote:

> 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
>

More information about the hotspot-runtime-dev mailing list