RFR 9: 8149750 Decouple sun.misc.Signal from the base module
Roger Riggs
Roger.Riggs at Oracle.com
Tue Feb 16 21:38:41 UTC 2016
Hi Chris,
Webrev updated in place:
http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/
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