RFR 9: 8149750 Decouple sun.misc.Signal from the base module
Roger Riggs
Roger.Riggs at Oracle.com
Wed Feb 17 15:49:05 UTC 2016
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.
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
>>>>
>>>>
>>
More information about the core-libs-dev
mailing list