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

David Holmes david.holmes at oracle.com
Wed Feb 17 08:33:48 UTC 2016


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

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 core-libs-dev mailing list