On 18/02/2016 11:54 AM, David Holmes wrote:
On 18/02/2016 1:49 AM, Roger Riggs wrote:
Hi David,
On 2/17/2016 3:33 AM, David Holmes wrote:
Hi Roger,
On 17/02/2016 7:37 AM, Roger Riggs wrote:
Hi David,
Webrev updated in place:
http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/
Thanks for those tweaks.
I'm still perplexed by the test. We established that for USR2, PIPE
and XFSZ the handler is not actually invoked - yet you are testing
that case. ??
I'll have to dig a bit more.
When that test is run, the SignalHandler does get called with the
expected signal.
The test succeeds on Linux with jdk 8 and jdk 9.
In my local testing (with the original java.util.Signal changes) raise()
throws IllegalStateException for USR2, PIPE and XFSZ (even though
register succeeded).
If I can get the time I'll try applying these exact changes and testing
them.
And everything worked as you said. I'm now extremely perplexed.
David
-----
Thanks,
David
Roger
David
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
