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

[David Holmes]

On 18/02/2016 11:54 AM, David Holmes wrote:
> 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.

And everything worked as you said. I'm now extremely perplexed.

David
-----

> 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
>>>>>>
>>>>>>
>>>>
>>