RFR 9: 8149750 Decouple sun.misc.Signal from the base module
david.holmes at oracle.com
Wed Feb 17 08:33:48 UTC 2016
On 17/02/2016 7:37 AM, Roger Riggs wrote:
> Hi David,
> Webrev updated in place:
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. ??
> 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
>>> 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).
>>> 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.
>> 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.
>> 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.
>> 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
>>> hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-9149750/
>> Hotspot changes are fine.
> Thanks, Roger
>>> Thanks, Roger
>>> JEP 260: https://bugs.openjdk.java.net/browse/JDK-8132928
More information about the core-libs-dev