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