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