RFR 9: 8149750 Decouple sun.misc.Signal from the base module
David Holmes
david.holmes at oracle.com
Mon Feb 15 04:55:46 UTC 2016
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?
---
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 *Handler.of method name reads strangely to me - "for" would be
better but I presume that is not allowed.
---
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 ??
---
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.
> hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-9149750/
Hotspot changes are fine.
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