Please, file a separate issue.
--Dmitry
-----Original Message-----
From: Yasumasa Suenaga <yasue...@gmail.com>
To: Peter Allwin <peter.all...@oracle.com>
Cc: Dmitry Samersoff <dmitry.samers...@oracle.com>,
"core-libs-dev@openjdk.java.net" <core-libs-dev@openjdk.java.net>,
"serviceability-...@openjdk.java.net" <serviceability-...@openjdk.java.net>
Sent: Sat, 06 Sep 2014 19:37
Subject: Re: RFR: JDK-8057556: JDP should better handle non-active interfaces
Hi all,
My patch works fine in my environment, however, JMX agent will be terminated
silently when I run command as following:
java -Dcom.sun.management.jmxremote.port=7091
-Dcom.sun.management.jmxremote.authenticate=false
-Dcom.sun.management.jmxremote.ssl=false
-Dcom.sun.management.jmxremote.autodiscovery=true
-Dcom.sun.management.jdp.source_addr=255.255.255.255 -version
Curently, sun.management.Agent#startDiscoveryService() throws
AgentConfigurationError
when JdpException is occurred. However, argument of AgentConfiguratonError
constructor
is incorrected. So NPE will be occurred. (I checked it with jdb.)
I think that we should fix it as following.
-----------
diff -r 68a6bb51cb26 src/java.management/share/classes/sun/management/Agent.java
--- a/src/java.management/share/classes/sun/management/Agent.java Mon Sep
01 13:33:28 2014 +0200
+++ b/src/java.management/share/classes/sun/management/Agent.java Sat Sep
06 23:50:58 2014 +0900
@@ -210,6 +210,8 @@
} else {
throw new AgentConfigurationError(INVALID_JMXREMOTE_PORT, "No
port specified");
}
+ } catch (JdpException e) {
+ error(e);
} catch (AgentConfigurationError err) {
error(err.getError(), err.getParams());
}
@@ -273,7 +275,7 @@
}
private static void startDiscoveryService(Properties props)
- throws IOException {
+ throws IOException, JdpException {
// Start discovery service if requested
String discoveryPort =
props.getProperty("com.sun.management.jdp.port");
String discoveryAddress =
props.getProperty("com.sun.management.jdp.address");
@@ -291,7 +293,7 @@
try{
shouldStart = Boolean.parseBoolean(discoveryShouldStart);
} catch (NumberFormatException e) {
- throw new AgentConfigurationError("Couldn't parse
autodiscovery argument");
+ throw new AgentConfigurationError(AGENT_EXCEPTION, "Couldn't
parse autodiscovery argument");
}
}
@@ -302,7 +304,7 @@
address = (discoveryAddress == null) ?
InetAddress.getByName(JDP_DEFAULT_ADDRESS) :
InetAddress.getByName(discoveryAddress);
} catch (UnknownHostException e) {
- throw new AgentConfigurationError("Unable to broadcast to
requested address", e);
+ throw new AgentConfigurationError(AGENT_EXCEPTION, e, "Unable
to broadcast to requested address");
}
int port = JDP_DEFAULT_PORT;
@@ -310,7 +312,7 @@
try {
port = Integer.parseInt(discoveryPort);
} catch (NumberFormatException e) {
- throw new AgentConfigurationError("Couldn't parse JDP port
argument");
+ throw new AgentConfigurationError(AGENT_EXCEPTION, "Couldn't
parse JDP port argument");
}
}
@@ -330,12 +332,7 @@
String instanceName =
props.getProperty("com.sun.management.jdp.name");
- try{
- JdpController.startDiscoveryService(address, port,
instanceName, jmxUrlStr);
- }
- catch(JdpException e){
- throw new AgentConfigurationError("Couldn't start JDP
service", e);
- }
+ JdpController.startDiscoveryService(address, port, instanceName,
jmxUrlStr);
}
}
-----------
I want also to contribute it.
Should I merge it to JDK-8057556? Or should I file to JBS as new issue?
Thanks,
Yasumasa
(2014/09/05 19:28), Yasumasa Suenaga wrote:
> Hi Peter,
>
> I fixed it and created new webrev.
> http://cr.openjdk.java.net/~ysuenaga/JDK-8057556/webrev.1/
>
> Could you review it again?
>
>
> Thanks,
>
> Yasumasa
>
>
> (2014/09/05 17:20), Peter Allwin wrote:
>> Looks like only the first Interface will be considered if no srcAddress is
>> provided (succeeded will be false and we will throw to exit the while loop).
>> Is this intended?
>>
>> Thanks!
>> /peter
>>
>>> On 4 sep 2014, at 17:59, Yasumasa Suenaga <yasue...@gmail.com> wrote:
>>>
>>> Hi all,
>>>
>>> Thank you so much, Dmitry!
>>>
>>> I've created webrev for it.
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8057556/webrev.0/
>>>
>>> Please review.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> (2014/09/04 21:26), Dmitry Samersoff wrote:
>>>> Yasumasa,
>>>>
>>>> The CR number is JDK-8057556
>>>>
>>>> I'll care about it's integration.
>>>>
>>>> -Dmitry
>>>>
>>>>> On 2014-09-02 18:52, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> I'm trying to use JDP on my Fedora20 machine.
>>>>> My machine has two NICs and only one NIC is up.
>>>>>
>>>>> I passed system properties as below, however JDP broadcaster
>>>>> thread was not started:
>>>>>
>>>>> -Dcom.sun.management.jmxremote.port=7091
>>>>> -Dcom.sun.management.jmxremote.authenticate=false
>>>>> -Dcom.sun.management.jmxremote.ssl=false
>>>>> -Dcom.sun.management.jmxremote.autodiscovery=true
>>>>> -Dcom.sun.management.jdp.name=TEST
>>>>>
>>>>> I checked exceptions with jdb, SocketException was occurred in
>>>>> JDPControllerRunner#run(), and it was caused by another NIC
>>>>> is down.
>>>>>
>>>>> Currently, DiagramChannel which is used in JDPBroadcaster
>>>>> tries to send JDP packet through all "UP" NICs.
>>>>> However, NIC which is controlled by NetworkManager seems to
>>>>> be still "UP" when ifdown command is executed.
>>>>> (It seems to be removed IP address from NIC only.)
>>>>>
>>>>>
>>>>> This problem may be Fedora, however I think it should be
>>>>> improved in JDK.
>>>>> I've created a patch as below, and it works fine in my environment.
>>>>> (jdk9/dev/jdk)
>>>>>
>>>>> If this patch may be accepted, I will file this to JBS.
>>>>>
>>>>> --------------------
>>>>> diff -r 68a6bb51cb26
>>>>> src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>>> ---
>>>>> a/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>>> Mon Sep 01 13:33:28 2014 +0200
>>>>> +++
>>>>> b/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>>> Tue Sep 02 23:25:50 2014 +0900
>>>>> @@ -35,6 +35,7 @@
>>>>> import java.nio.ByteBuffer;
>>>>> import java.nio.channels.DatagramChannel;
>>>>> import java.nio.channels.UnsupportedAddressTypeException;
>>>>> +import java.util.Enumeration;
>>>>>
>>>>> /**
>>>>> * JdpBroadcaster is responsible for sending pre-built JDP packet
>>>>> across a Net
>>>>> @@ -79,6 +80,15 @@
>>>>> if (srcAddress != null) {
>>>>> // User requests particular interface to bind to
>>>>> NetworkInterface interf =
>>>>> NetworkInterface.getByInetAddress(srcAddress);
>>>>> +
>>>>> + if (interf == null) {
>>>>> + throw new JdpException("Unable to get network interface
>>>>> for " + srcAddress.toString());
>>>>> + }
>>>>> +
>>>>> + if (!interf.isUp() || !interf.supportsMulticast()) {
>>>>> + throw new JdpException(interf.getName() + " does not
>>>>> support multicast.");
>>>>> + }
>>>>> +
>>>>> try {
>>>>> channel.bind(new InetSocketAddress(srcAddress, 0));
>>>>> } catch (UnsupportedAddressTypeException ex) {
>>>>> @@ -86,6 +96,23 @@
>>>>> }
>>>>> channel.setOption(StandardSocketOptions.IP_MULTICAST_IF,
>>>>> interf);
>>>>> }
>>>>> + else {
>>>>> + Enumeration<NetworkInterface> nics =
>>>>> NetworkInterface.getNetworkInterfaces();
>>>>> + while (nics.hasMoreElements()) {
>>>>> + NetworkInterface nic = nics.nextElement();
>>>>> +
>>>>> + if (nic.isUp() && nic.supportsMulticast()) {
>>>>> + try {
>>>>> +
>>>>> channel.setOption(StandardSocketOptions.IP_MULTICAST_IF, nic);
>>>>> + } catch (IOException ex) {
>>>>> + System.err.println("WARNING: JDP broadcaster
>>>>> cannot use " + nic.getName() + ": " + ex.getMessage());
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + }
>>>>> +
>>>>> + }
>>>>> +
>>>>> }
>>>>>
>>>>> /**
>>>>> --------------------
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>
>>>>
