Hi Severin,
Thanks for looking into this!
On 25/02/2019 15:31, Severin Gehwolf wrote:
Hi Daniel,
Thanks for the review!
Latest webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/02/webrev/
On Fri, 2019-02-22 at 18:31 +0000, Daniel Fuchs wrote:
Hi Severin,
Did you manage to make the test pass locally?
Yes. Here is the latest run:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/02/JMXInterfaceBindingTest.jtr
FWIW, it passed locally with the v1 change proposed. But then again, it
didn't do what it was intended for: Try to bind to multiple host:port
pairs on a single system. This didn't work prior JDK-6425769 as it
would bind to *all* interfaces. The updated webrev tries to remediate
this.
Thanks - I experimented with similar changes too.
This test has had a long history of failing, and I've come
to suspect that it might be flawed.
If it has a history of failing, why aren't there any bugs for it? How
did it fail? Are you referring to JDK-8145982 and JDK-8146015?
Yes - well agreed - long history might have been a bit exaggerated.
First - it has /timeout=5. I believe that's much too short.
It immediately failed the first time I ran it locally on my machine.
I'm not sure we need a larger timeout. It's 20 seconds on my machine
which seems more than enough. What makes you think we'd need to get
this increased and it would actually do anything for test stability?
Experience - and the fact that I saw it fail in timeout on my
local machine (albeit intermittently).
The test system runs tests with many different configurations,
some of them with various combination like -Xcomp or -Xint
which tend to have some measurable effects on JVM startup.
Tests that do compilation or start sub-processes are often
more sensitive to these option sets.
Usually, I would recommend not setting the timeout at all -
unless proven that the default jtreg timeout is too short.
Especially for those cases where the test is not supposed to
timeout.
Then it uses well known ports. That's bad. There's no guarantee
that these ports will be free.
Fair enough. Fixed in the latest webrev. Now picks a random port in
range [9100, 9200].
Thanks.
Then - it only use addresses configured for "localhost".
Can there ever be more than one such address?
Yes, a multi-homed computer could use this for example this in
/etc/hosts on Linux:
192.168.1.18 localhost
192.168.122.1 localhost
OK - thanks. Not sure how much stable that would be. Ideally we
should use port 0 - but then retrieving the actual port might
not be easy. Let's wait until we see the test fail.
Looks like this on my system:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/02/JMXInterfaceBindingTest-multihome.jtr
If there is more
than one, I suspect the test will fail: I believe the subprocess
started by the test would need the additional
-Djava.rmi.server.hostname=<adddress>
That's needed for 127.0.0.1 (loopback) support, so I've added it. Other
IP addresses don't seem to need it. So the test does not need it per
se, but it's useful so that more configs can actually test this.
on the command line.
And even with that - it might still fail if those two addresses
can't be bound simultaneously on the same port.
No. Socket addresses are unique per host:port pair. If that wasn't
possible, the whole test would be pointless.
Network tests that use "localhost" and listen to 0.0.0.0 are
often more prone to intermittent failures. I think this is due
to the policy of reusing addresses and ports of the
underlying OS. This test doesn't bind to the wildcard address
however - so we can hope it will not fall into this category.
Then why are loopback addresses filtered out? Because of two
things I guess:
- if several loopback are defined, I'm not sure you can
bind them on the same port (and it might be difficult to
figure that out)
- if you bind to the loopback address, then you most probably
need to start the subprocess with
-Djava.rmi.server.hostname=<loopback-adddress>
Yes. I've tried to avoid those issues by allowing one non-loopback
address and 127.0.0.1 explicitly. Is that a reasonable assumption?
I believe so.
If we don't see this test failing, I suspect this is because
it is either never selected, or always passes trivially.
If we start selecting it - or have a configuration where it
doesn't trivially pass, I suspect it will fail.
My bet is on it passing trivially with:
"Ignoring manual test since no more than one IPs are configured for 'localhost'"
Well - I'm going to send your patch through our internal
test system - I'll let you know of the result.
best regards,
-- daniel
Thanks,
Severin
On 22/02/2019 15:04, Severin Gehwolf wrote:
Hi!
Bug: https://bugs.openjdk.java.net/browse/JDK-8219585
Could somebody please review this trivial testbug. For a config like
[1] the logic prior JDK-8145982 returned this list for
getAddressesForLocalHost():
[localhost/127.0.0.1, localhost/192.168.1.18, localhost/0:0:0:0:0:0:0:1]
Post JDK-8145982, getAddressesForLocalHost() returns:
[localhost/192.168.1.18]
The fix is trivial. Just adjust the condition for as to when the test
should actually trivially pass:
diff --git
a/test/jdk/sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java
b/test/jdk/sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java
--- a/test/jdk/sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java
+++ b/test/jdk/sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java
@@ -176,8 +176,8 @@
}
public static void main(String[] args) {
- List<InetAddress> addrs = getAddressesForLocalHost();
- if (addrs.size() < 2) {
+ List<InetAddress> addrs = getNonLoopbackAddressesForLocalHost();
+ if (addrs.size() < 1) {
System.out.println("Ignoring manual test since no more than one IPs
are configured for 'localhost'");
return;
}
@@ -186,7 +186,7 @@
System.out.println("All tests PASSED.");
}
- private static List<InetAddress> getAddressesForLocalHost() {
+ private static List<InetAddress> getNonLoopbackAddressesForLocalHost() {
try {
return NetworkInterface.networkInterfaces()
Testing: Manual testing on local setup. jdk/submit (currently running)
Thanks,
Severin
[1] $ grep localhost /etc/hosts | grep -v '::'
127.0.0.1 localhost localhost.localdomain localhost4 localhost4.localdomain4
192.168.1.18 localhost
