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

Roger Riggs Roger.Riggs at Oracle.com
Tue Feb 16 21:37:59 UTC 2016


Hi David,

Webrev updated in place:
    http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/

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