Hi Holmes,
I agree with your comment on duplicated comment. It should be
factored out.
The context of the change is that the group of signal handlers should
be registered at best effort, which means for all JVM, we should try to
register them all in an independent way. In fact, the original code
does not act homogeneously. For example, in
solaris/classes/java/lang/Terminator.java, there are 3 registration
calls for signal "HUP", "INT" and "TERM". On start up of some JVM,
"INT" may already have been occupied for some reason and this will cause
"TERM" to be skipped. It is not what the original code intends to do,
is it?
I provided a new revision of patch to remove the dup comment,
available @ http://cr.openjdk.java.net/~youdwei/ojdk-430/webrev.01/.
Your comment is much appreciated.
Thanks and best regards,
Frank
On 8/6/2012 9:51 AM, David Holmes wrote:
Hi Frank,
On 3/08/2012 5:39 PM, Frank Ding wrote:
Hi guys,
I found that in java.lang.Terminator, setup() method,
The following code of registering default signal handlers can be
improved:
/ try {
Signal.handle(new Signal("INT"), sh);
Signal.handle(new Signal("TERM"), sh);
} catch (IllegalArgumentException e) {
}/
The revised code is illustrated below:
/ try {
Signal.handle(new Signal("INT"), sh);
} catch (IllegalArgumentException e) {
}
try {
Signal.handle(new Signal("TERM"), sh);
} catch (IllegalArgumentException e) {
}
/The improved version makes more sense since exception thrown from first
Signal.handle call does not affect subsequent calls. This is more
consistent with its original intention.
A patch I made is available @
http://cr.openjdk.java.net/~youdwei/ojdk-430/webrev.00//
/Could anybody please take a look at it? Thanks in advance/
Can you explain the context for this change. It seems to me that there
is an expectation that the group of signals act homogenously: all or
none, whereas your change make it appear that the success/failure for
each signal is independent. Understanding exactly when/why the
IllegalArgumentException is thrown is important here.
I don't like seeing the duplicated comment, perhaps that can be
factored out to the head of the block of code?
Thanks,
David Holmes
Best regards,
Frank
/
