RFR 9: 8149750 Decouple sun.misc.Signal from the base module
Chris Hegarty
chris.hegarty at oracle.com
Thu Feb 18 11:22:16 UTC 2016
On 16 Feb 2016, at 21:38, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
> Hi Chris,
>
> Webrev updated in place:
> http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/
Thanks Roger, I’m happy with the source changes.
-Chris.
> On 2/15/2016 6:22 AM, Chris Hegarty wrote:
>> On 12/02/16 17:47, 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/
>>
>> Overall looks ok, and satisfies the requirement of JEP 260.
>>
>> It took me a while to satisfy myself that it is ok to "ignore"
>> the passed Signal in the wrapper's 'handle' methods. The assumption
>> is that this wrapper's signal field and the passes signal are, MUST,
>> represent the same underlying signal. Maybe an assert to make this
>> explicit?
> The jdk.internal.misc.Signal passed to the wrapper's methods needs to be
> mapped to the corresponding sun.misc.Signal but instead of maintaining a map
> the s.m.Signal instance is kept in the wrapper of the s.m.SignalHandler.
>>
>> Looking at j.i.m.Signal.<init>, I can see that you explicitly check
>> for the 'SIG' prefix and prepend it if not there, but toString() also
>> prepends it. Will getName also be impacted by this ( it may now have
>> the name prepended with 'SIG', where it previously was not ).
> Yes, there was a bug there; reverting to backward compatible behavior.
> HotSpot recognizes different forms of signal names on Windows and Posix. [1]
> To be compatible with previous versions, the "SIG" prefix allowed by the HotSpot change
> (only on Posix) is not supported and throws IllegalArgumentException.
>
> Thanks, Roger
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8149748
>
>>
>>> hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-9149750/
>>
>> Looks fine.
>>
>> -Chris.
>
More information about the hotspot-runtime-dev
mailing list