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