On 10/23/13 3:13 PM, Mandy Chung wrote:
On 10/21/2013 5:23 PM, Stuart Marks wrote:
Please review the following spec change (deprecation) and change of a property
default value.
Briefly, RMI has some machinery that will attempt to tunnel RMI requests
through HTTP under certain circumstances. This is a maintenance burden and
also causes customer confusion. The change here deprecates the HTTP tunneling
mechanism and changes a property to disable HTTP tunneling by default.
...
Webrev:
http://cr.openjdk.java.net/~smarks/reviews/8023862/webrev.0/
This change looks fine. Just minor comments:
Hi Mandy,
Thanks for taking a look at this.
java/rmi/server/RMISocketFactory.java
47 * value to {@code false} will re-enable the HTTP tunneling mechanisms.
nit: In the context of a specification, would it be appropriate to say "enable"
instead of "re-enable"?
Yes, good suggestion, saying just "enable" reads better here. Will fix.
sun/rmi/transport/proxy/RMIMasterSocketFactory.java
line 120-123 which is existing code: is this catch block redundant?
It's quite possibly redundant. It's hard to see how the code in the try block
could throw an exception, but if one is thrown, it needs to be caught in order
for the constructor to complete normally.
But setting the setFactories variable to false in the catch block is certainly
unnecessary, since setFactories is only set to true -- if it's set at all -- at
the very end of the try-block. I'll remove this and clarify the comment.
I assume that separate bug will be filed to update the RMI documentations.
Yes, I have a bunch of pending items to update the corresponding documentation,
which will be done separately from the code and javadoc updates here.
Mandy
Thanks!
s'marks
