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

[Dmitry Samersoff]

Yumin,

(* fixed with correct signal name *)

1.

We can do:

  if (isatty(fileno(stdin))) {
     DO_SIGNAL_CHECK(SHUTDOWN2_SIGNAL);
  }

instead of changing macro

2.
or we can do:

 case SHUTDOWN2_SIGNAL:
    if ( thisHandler == (address) user_handler()
         || thisHandler == CAST_FROM_FN_PTR(address, SIG_IGN) ) {

       // take handler OK path
    }

as I think it's safe in all cases to have SIGINT ignored.


3.

isatty() should return False (didn't check it though) if stdin is closed
and reopen as a regular file before launching java. (i.e. someone
launched java from other process or create VM directly)

So I'm for option (2) above.

Also it might be faster to use fstat() and check whether st_mode ==
S_ISCHR.


-Dmitry


On 2014-03-14 20:27, 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.