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


Reply via email to