RFR 9: 8149750 Decouple sun.misc.Signal from the base module

[Chris Hegarty]

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. 
>