RFR 9: 8149750 Decouple sun.misc.Signal from the base module

David Holmes david.holmes at oracle.com
Thu Feb 18 01:54:25 UTC 2016


On 18/02/2016 1:49 AM, Roger Riggs wrote:
> Hi David,
>
> On 2/17/2016 3:33 AM, David Holmes wrote:
>> Hi Roger,
>>
>> On 17/02/2016 7:37 AM, Roger Riggs wrote:
>>> Hi David,
>>>
>>> Webrev updated in place:
>>> http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/
>>
>> Thanks for those tweaks.
>>
>> I'm still perplexed by the test. We established that for USR2, PIPE
>> and XFSZ the handler is not actually invoked - yet you are testing
>> that case. ??
> I'll have to dig a bit more.
> When that test is run, the SignalHandler does get called with the
> expected signal.
> The test succeeds on Linux with jdk 8 and jdk 9.

In my local testing (with the original java.util.Signal changes) raise() 
throws IllegalStateException for USR2, PIPE and XFSZ (even though 
register succeeded).

If I can get the time I'll try applying these exact changes and testing 
them.

Thanks,
David

> Roger
>
>>
>> David
>>
>>> On 2/14/2016 11:55 PM, David Holmes wrote:
>>>> Hi Roger,
>>>>
>>>> A few mostly minor comments ...
>>>>
>>>> On 13/02/2016 3:47 AM, Roger Riggs wrote:
>>>>> Please review moving the functionality of sun.misc.Signal to
>>>>> jdk.internal.misc and
>>>>> creating a wrapper sun.misc.Signal for existing external uses.
>>>>>
>>>>> +++ This time including the hotspot changes to update the target of
>>>>> the
>>>>> upcalls.
>>>>>
>>>>> A new replacement API will be considered separately.
>>>>>
>>>>> The update includes a test that passes with or without the changes.
>>>>> (Except for an NPE instead of SEGV if null is passed).
>>>>>
>>>>> Webrev:
>>>>>    jdk: http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/
>>>>
>>>> I take it we don't care about reorder files any more?
>>> There does not seem to be any particular guidance about how/when to use
>>> them on Solaris.
>>> So it seemed as reasonable to remove the entries as to update them.
>>>>
>>>> ---
>>>>
>>>> src/java.base/share/classes/sun/misc/Signal.java
>>>>
>>>> The @since should not be changed to 9. You only changed the
>>>> implementation not the API. Conversely the renamed class should
>>>> arguably be @since 9 and not @since 1.2. But it is wrong to say
>>>> sun.misc.Signal is @since 9.
>>> The new sun.misc.Signal is a wrapper around the original implementation
>>> now in jdk.internal.misc.
>>> I had modified the mercurial commands to rename sun.misc.Signal to
>>> jdk.internal.misc to preserve the history
>>> of the implementation.
>>>
>>> I will correct sun.misc.Signal to retain the original @since and use
>>> @since 9 for the jdk.internal.misc
>>>>
>>>> The *Handler.of method name reads strangely to me - "for" would be
>>>> better but I presume that is not allowed.
>>> The xx.of ()  pattern has been used recently.
>>>>
>>>> ---
>>>>
>>>> src/java.base/share/classes/jdk/internal/misc/Signal.java
>>>>
>>>> Not sure I understand the role of NativeSignalHandler any more - given
>>>> it can't actually be used ??
>>> They are used to retain the previous handler and can be used to restore
>>> that handler.
>>> The ability to invoke the handler directly was removed as a
>>> simplification.
>>>>
>>>> ---
>>>>
>>>> test/sun/misc/SunMiscSignalTest.java
>>>>
>>>> Based on our email discussion this test can not be testing that the
>>>> handler actually gets invoked - as we know it does not in some cases.
>>>> I have doubts that sun.misc.Signal ever intended to support all the
>>>> signals you are testing.
>>> There were no tests for sun.misc.Signal.  I created the tests based on
>>> the current implementations.
>>> They can be updated to reflect current support for handle, raise, and
>>> whether a signal handler is called
>>> and of course -Xrs.  It should allow detection of unintended changes in
>>> behavior.
>>>
>>>>
>>>>> hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-9149750/
>>>>
>>>> Hotspot changes are fine.
>>>
>>> Thanks, Roger
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Issue:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8149750
>>>>>
>>>>> Thanks, Roger
>>>>>
>>>>> JEP 260: https://bugs.openjdk.java.net/browse/JDK-8132928
>>>>>
>>>>>
>>>
>


More information about the hotspot-runtime-dev mailing list