RFR: 8232571: Add missing SIGINFO signal

Roger Riggs roger.riggs at oracle.com
Tue Oct 22 13:43:35 UTC 2019


This looks fine.

Thanks, Roger

On 10/21/19 2:40 AM, David Holmes wrote:
> Hi Benoit,
>
> On 21/10/2019 9:52 pm, Benoit Daloze wrote:
>> My apologies, here it is.
>
> The mailing list strips many/most attachments, but as I was cc'd 
> directly I got it. I've posted a webrev for others:
>
> http://cr.openjdk.java.net/~dholmes/8232571/webrev/
>
> This all seems fine. I can sponsor this for you. I'll give Roger a 
> chance to comment further before pushing it.
>
> Thanks,
> David
>
>> On 10/21/19 1:23 PM, David Holmes wrote:
>>> Hi Benoit,
>>>
>>> Your patch was stripped from the email.
>>>
>>> David
>>>
>>> On 21/10/2019 7:50 pm, Benoit Daloze wrote:
>>>> Hello,
>>>>
>>>> Here is an updated patch fixing the copyright year, and adding the
>>>> {"INFO", IsSupported.NO, CanRegister.NO, CanRaise.NO, Invoked.NO}
>>>> line in test/jdk/sun/misc/SunMiscSignalTest.java
>>>>
>>>> Benoit
>>>>
>>>> On 10/21/19 6:07 AM, David Holmes wrote:
>>>>> Hi Roger,
>>>>>
>>>>> On 21/10/2019 1:43 pm, Roger Riggs wrote:
>>>>>> Hi,
>>>>>>
>>>>>> If I understand correctly  SIGINFO is only defined on OSX/BSD and
>>>>>> would not be implemented on any other other OS.
>>>>>>
>>>>>> SIGINFO is added to a list of signals that are only included if 
>>>>>> the OS is OSX. (See line 166).
>>>>>
>>>>> The new test is distinct from the modified test. The new test is 
>>>>> defined as:
>>>>>
>>>>> +/*
>>>>> + * @test
>>>>> + * @requires os.family != "windows" & os.family != "aix"
>>>>> + *
>>>>> + * @summary tests the SIGINFO signal (available on BSD platforms)
>>>>> + * VM testbase keywords: [signal, runtime, linux, solaris, macosx]
>>>>> + *
>>>>> + * @library /test/lib
>>>>> + * @run main/native SigTestDriver SIGINFO
>>>>> + */
>>>>>
>>>>> This will attempt to test SIGINFO on Solaris and Linux as well as 
>>>>> OS X.
>>>>>
>>>>> However I've checked and this unknown signal will just be ignored 
>>>>> anyway and the test will claim to pass. It would seem better to me 
>>>>> that it is excluded from all platforms that don't support it ... 
>>>>> though it seems that isn't what we do with these signal tests as 
>>>>> we also test Solaris only signals on Linux and BSD/OSX. So this is 
>>>>> consistent with the existing tests.
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>
>>>>>>
>>>>>> A bit more test code would be needed to verify it was NOT 
>>>>>> implemented on other platforms.
>>>>>> It could be added to the posixNonOSXSignals list with:
>>>>>>
>>>>>> {"INFO", IsSupported.NO, CanRegister.NO, CanRaise.NO, Invoked.NO},
>>>>>>
>>>>>> (And some testing)
>>>>>>
>>>>>> + Copyright.
>>>>>>
>>>>>> LGTM
>>>>>>
>>>>>> Roger
>>>>>>
>>>>>>
>>>>>> On 10/20/19 7:10 PM, David Holmes wrote:
>>>>>>> Hi Benoit,
>>>>>>>
>>>>>>> On 19/10/2019 1:27 am, Benoit Daloze wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Please review the following fix for 
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8232571
>>>>>>>>
>>>>>>>> In short, new sun.misc.Signal("INFO") throws 
>>>>>>>> IllegalArgumentException on JDK11+
>>>>>>>> due to the signal not being in the known signal list. This is a 
>>>>>>>> regression, it worked fine in JDK8.
>>>>>>>>
>>>>>>>> I attach a patch to fix this,.
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8232571
>>>>>>>
>>>>>>> The functional change seems fine to restore support for SIGINFO. 
>>>>>>> I'm still a bit perplexed as to when exactly it got removed. :)
>>>>>>>
>>>>>>> I don't understand how the new test can pass on any platform 
>>>>>>> except BSD/OSX ?
>>>>>>>
>>>>>>> Please update the copyright year in 
>>>>>>> test/jdk/sun/misc/SunMiscSignalTest.java.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Benoit Daloze
>>>>>>



More information about the hotspot-runtime-dev mailing list