Hi Harsha,

On 27.1.2016 15:30, Harsha Wardhana B wrote:
Hi Jaroslav,

Below is the updated webrev with minor line alignment fixes.

http://cr.openjdk.java.net/~hb/8031753/webrev.01/
<http://cr.openjdk.java.net/%7Ehb/8031753/webrev.01/>

The issue can be reproduced by removing entry for hostname in
/etc/hosts (for linux) and running
'javax/management/remote/mandatory/connection/RMIConnectionIdTest'
test.
Hence no new jtreg test case have been written.

Agreed that we are putting involved logic into a container object. The
alternative would be remove all the involved logic from JMXServiceURL
and create new a helper classes that can do URL validation. But that
would be a API/Design change that can break compatibility with existing
applications. Since the bug is not critical, I think we can make do with
adding little more involved logic into existing validations.

I see. Probably this is acceptable.


I would welcome more comments and discussions around this change. Please
do review and let me know your thoughts.

(R)eviewed.

Thanks!

-JB-


Thanks
Harsha

On Wednesday 27 January 2016 03:47 PM, Jaroslav Bachorik wrote:
Hi Harsha,

On 27.1.2016 11:09, Harsha Wardhana B wrote:
Hello All,

For the below fix, I have made appropriate javadoc changes and uploaded
new webrev at below location.

http://cr.openjdk.java.net/~hb/8031753/webrev.00/
<http://cr.openjdk.java.net/%7Ehb/8031753/webrev.00/>

Please review and let me know your comments.

I'm a bit uneasy by introducing rather involved logic into supposedly
simple value holder type JMXServiceURL. On the other hand, there
already are some pieces of non-trivial code to deduce defaults there
so we might just continue on this path.

Here are some nits:

src/java.management/share/classes/javax/management/remote/JMXServiceURL.java

L309-319: Indentation seem to be corrupted here
L365-366: The opening curly bracket should be on the same line

Missing reg test.

Cheers,

-JB-


Thanks
Harsha

On Thursday 14 January 2016 09:34 AM, Harsha Wardhana B wrote:
Gentle reminder :)

On Monday 11 January 2016 02:51 PM, Jaroslav Bachorik wrote:
[adding JMX dev list]

On 8.1.2016 10:44, Harsha Wardhana B wrote:
Hi All,

Please review the fix for,

Issue : JDK-8031753 <http://JDK-8031753> - JMXServiceURL should
not use
getLocalHost or its usage should be enhanced
Webrev :
http://cr.openjdk.java.net/~jbachorik/sponsorship/8031753/webrev.00/

The issue can be reproduced by removing entry for hostname in
/etc/hosts
(for linux) and running
'javax/management/remote/mandatory/connection/RMIConnectionIdTest'
test.
Hence no new jtreg test case have been written.

Fix Description:

If JMXServiceURL is given null host, it tries to resolve hostname via
InetAddress.getLocalHost() and if that fails throws
MalformedURLException. Since JMX Agent will listen on all interfaces
(0.0.0.0) if host is null, the fix handles the exception and assigns
first active non-loopback interface IP address to host. That way
we can
have JMXServiceURL with IP address instead of hostname.

It is possible that we might end up constructing JMXServiceURL
with IP
for an interface which clients may not be able to connect (because of
network reachability issues). In that case, the recommendation
would be
to try to start JMX Agent with
com.sun.management.jmxremote.host=xx.xx.xx.xx option. That way, the
agent will be started on interface that clients can actually connect
to.

Please review the fix and let me know your comments. The fix requires
amending javadoc with above behavioral change and a CCC review as
well
which I will send out later.

Thanks
Harsha







Reply via email to