RFR: 8232571: Add missing SIGINFO signal

David Holmes david.holmes at oracle.com
Tue Oct 22 06:40:51 UTC 2019


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