From 9b4a8ec37805be3eabf3a4b443bbdb692ffa2608 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 29 Sep 2011 16:53:00 -0700 Subject: Fix a couple of isReachable bugs. We shouldn't throw NPE if you call isReachable on a deserialized InetAddress. Fixed by removing the "globals", which also fixes the unreported bug that calling isReachable on the same InetAddress was not thread-safe. Bug: http://code.google.com/p/android/issues/detail?id=20203 Also, the arguments to isReachableByTCP in isReachable(NetworkInterface, int, int) were the wrong way round, which meant we'd always return false (unless you were asking if localhost was reachable). Bug: http://code.google.com/p/android/issues/detail?id=20107 Bug: 2497441 Bug: 3213503 Change-Id: Ic808e774c28be6487e30e6acb8bc06f766f5c71d --- luni/src/main/java/java/net/InetAddress.java | 131 ++++++--------------- .../java/libcore/java/net/InetAddressTest.java | 20 +++- .../AttributedCharacterIteratorAttributeTest.java | 12 +- .../java/libcore/java/util/OldCollectionsTest.java | 14 +-- .../libcore/java/util/OldPriorityQueueTest.java | 13 +- 5 files changed, 68 insertions(+), 122 deletions(-) (limited to 'luni/src') diff --git a/luni/src/main/java/java/net/InetAddress.java b/luni/src/main/java/java/net/InetAddress.java index 4aeb1b0..c12a350 100644 --- a/luni/src/main/java/java/net/InetAddress.java +++ b/luni/src/main/java/java/net/InetAddress.java @@ -31,10 +31,12 @@ import java.util.Collections; import java.util.Comparator; import java.util.Enumeration; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; import libcore.io.ErrnoException; import libcore.io.GaiException; -import libcore.io.Libcore; import libcore.io.IoBridge; +import libcore.io.Libcore; import libcore.io.Memory; import libcore.io.StructAddrinfo; import static libcore.io.OsConstants.*; @@ -133,12 +135,6 @@ public class InetAddress implements Serializable { private static final long serialVersionUID = 3286316764910316507L; - private transient Object waitReachable = new Object(); - - private boolean reached; - - private int addrCount; - private int family; byte[] ipaddress; @@ -641,8 +637,8 @@ public class InetAddress implements Serializable { /** * Tries to reach this {@code InetAddress}. This method first tries to use - * ICMP (ICMP ECHO REQUEST). When first step fails, a TCP connection - * on port 7 (Echo) of the remote host is established. + * ICMP (ICMP ECHO REQUEST), falling back to a TCP connection + * on port 7 (Echo) of the remote host. * * @param timeout * timeout in milliseconds before the test fails if no connection @@ -660,8 +656,8 @@ public class InetAddress implements Serializable { /** * Tries to reach this {@code InetAddress}. This method first tries to use - * ICMP (ICMP ECHO REQUEST). When first step fails, a TCP connection - * on port 7 (Echo) of the remote host is established. + * ICMP (ICMP ECHO REQUEST), falling back to a TCP connection + * on port 7 (Echo) of the remote host. * * @param networkInterface * the network interface on which to connection should be @@ -678,112 +674,53 @@ public class InetAddress implements Serializable { * @throws IllegalArgumentException * if ttl or timeout is less than zero. */ - public boolean isReachable(NetworkInterface networkInterface, final int ttl, - final int timeout) throws IOException { + public boolean isReachable(NetworkInterface networkInterface, final int ttl, final int timeout) throws IOException { if (ttl < 0 || timeout < 0) { throw new IllegalArgumentException("ttl < 0 || timeout < 0"); } + + // The simple case. if (networkInterface == null) { - return isReachableByTCP(this, null, timeout); - } else { - return isReachableByMultiThread(networkInterface, ttl, timeout); + return isReachable(this, null, timeout); } - } - /* - * Uses multi-Thread to try if isReachable, returns true if any of threads - * returns in time - */ - private boolean isReachableByMultiThread(NetworkInterface netif, - final int ttl, final int timeout) - throws IOException { - List addresses = Collections.list(netif.getInetAddresses()); - if (addresses.isEmpty()) { + // Try each NetworkInterface in parallel. + // Use a thread pool Executor? + List sourceAddresses = Collections.list(networkInterface.getInetAddresses()); + if (sourceAddresses.isEmpty()) { return false; } - reached = false; - addrCount = addresses.size(); - boolean needWait = false; - for (final InetAddress addr : addresses) { - // loopback interface can only reach to local addresses - if (addr.isLoopbackAddress()) { - Enumeration NetworkInterfaces = NetworkInterface - .getNetworkInterfaces(); - while (NetworkInterfaces.hasMoreElements()) { - NetworkInterface networkInterface = NetworkInterfaces - .nextElement(); - Enumeration localAddresses = networkInterface - .getInetAddresses(); - while (localAddresses.hasMoreElements()) { - if (InetAddress.this.equals(localAddresses - .nextElement())) { - return true; - } - } - } - - synchronized (waitReachable) { - addrCount--; - - if (addrCount == 0) { - // if count equals zero, all thread - // expired,notifies main thread - waitReachable.notifyAll(); - } - } - continue; - } - - needWait = true; + final InetAddress destinationAddress = this; + final CountDownLatch latch = new CountDownLatch(sourceAddresses.size()); + final AtomicBoolean isReachable = new AtomicBoolean(false); + for (final InetAddress sourceAddress : sourceAddresses) { new Thread() { @Override public void run() { - /* - * Spec violation! This implementation doesn't attempt an - * ICMP; it skips right to TCP echo. - */ - boolean threadReached = false; try { - threadReached = isReachableByTCP(addr, InetAddress.this, timeout); - } catch (IOException e) { - } - - synchronized (waitReachable) { - if (threadReached) { - // if thread reached this address, sets reached to - // true and notifies main thread - reached = true; - waitReachable.notifyAll(); - } else { - addrCount--; - if (addrCount == 0) { - // if count equals zero, all thread - // expired,notifies main thread - waitReachable.notifyAll(); + if (isReachable(destinationAddress, sourceAddress, timeout)) { + isReachable.set(true); + // Wake the main thread so it can return success without + // waiting for any other threads to time out. + while (latch.getCount() > 0) { + latch.countDown(); } } + } catch (IOException ignored) { } + latch.countDown(); } }.start(); } - - if (needWait) { - synchronized (waitReachable) { - try { - while (!reached && (addrCount != 0)) { - // wait for notification - waitReachable.wait(1000); - } - } catch (InterruptedException e) { - // do nothing - } - return reached; - } + try { + latch.await(); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); // Leave the interrupted bit set. } - - return false; + return isReachable.get(); } - private boolean isReachableByTCP(InetAddress destination, InetAddress source, int timeout) throws IOException { + private boolean isReachable(InetAddress destination, InetAddress source, int timeout) throws IOException { + // TODO: try ICMP first (http://code.google.com/p/android/issues/detail?id=20106) FileDescriptor fd = IoBridge.socket(true); boolean reached = false; try { diff --git a/luni/src/test/java/libcore/java/net/InetAddressTest.java b/luni/src/test/java/libcore/java/net/InetAddressTest.java index 1ebd7ec..36ea464 100644 --- a/luni/src/test/java/libcore/java/net/InetAddressTest.java +++ b/luni/src/test/java/libcore/java/net/InetAddressTest.java @@ -16,10 +16,19 @@ package libcore.java.net; -import java.net.InetAddress; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.net.Inet4Address; import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.NetworkInterface; import java.net.UnknownHostException; +import java.util.Collections; +import tests.util.SerializationTester; public class InetAddressTest extends junit.framework.TestCase { private static final byte[] LOOPBACK6_BYTES = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1 }; @@ -127,6 +136,15 @@ public class InetAddressTest extends junit.framework.TestCase { assertTrue(InetAddress.getByName("ff15::").isMCSiteLocal()); } + public void test_isReachable() throws Exception { + // http://code.google.com/p/android/issues/detail?id=20203 + InetAddress addr = SerializationTester.getDeserializedObject(InetAddress.getByName("www.google.com")); + addr.isReachable(500); + for (NetworkInterface nif : Collections.list(NetworkInterface.getNetworkInterfaces())) { + addr.isReachable(nif, 20, 500); + } + } + public void test_isSiteLocalAddress() throws Exception { assertFalse(InetAddress.getByName("144.32.32.1").isSiteLocalAddress()); assertTrue(InetAddress.getByName("10.0.0.1").isSiteLocalAddress()); diff --git a/luni/src/test/java/libcore/java/text/AttributedCharacterIteratorAttributeTest.java b/luni/src/test/java/libcore/java/text/AttributedCharacterIteratorAttributeTest.java index 9119582..cca00f5 100644 --- a/luni/src/test/java/libcore/java/text/AttributedCharacterIteratorAttributeTest.java +++ b/luni/src/test/java/libcore/java/text/AttributedCharacterIteratorAttributeTest.java @@ -27,6 +27,7 @@ import java.text.AttributedCharacterIterator; import java.text.DateFormat; import java.text.NumberFormat; import junit.framework.TestCase; +import tests.util.SerializationTester; /** * AttributedCharacterIterator.Attribute is used like the base enum type and @@ -44,21 +45,14 @@ public final class AttributedCharacterIteratorAttributeTest extends TestCase { public void testSerializingSubclass() throws IOException, ClassNotFoundException { AttributedCharacterIterator.Attribute a = new CustomAttribute(); try { - reserialize(a); + SerializationTester.getDeserializedObject(a); fail(); } catch (InvalidObjectException expected) { } } private void assertSameReserialized(Object o) throws ClassNotFoundException, IOException { - assertSame(o, reserialize(o)); - } - - private Object reserialize(Object o) throws IOException, ClassNotFoundException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - new ObjectOutputStream(out).writeObject(o); - InputStream in = new ByteArrayInputStream(out.toByteArray()); - return new ObjectInputStream(in).readObject(); + assertSame(o, SerializationTester.getDeserializedObject(o)); } private static class CustomAttribute extends AttributedCharacterIterator.Attribute { diff --git a/luni/src/test/java/libcore/java/util/OldCollectionsTest.java b/luni/src/test/java/libcore/java/util/OldCollectionsTest.java index 17dca67..f54fc56 100644 --- a/luni/src/test/java/libcore/java/util/OldCollectionsTest.java +++ b/luni/src/test/java/libcore/java/util/OldCollectionsTest.java @@ -606,41 +606,41 @@ public class OldCollectionsTest extends TestCase { public void test_checkedCollectionSerializationCompatability() throws Exception { Collection c = Collections.emptySet(); c = Collections.checkedCollection(c, String.class); - SerializationTester.assertCompabilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedCollection.golden.ser"); + SerializationTester.assertCompatibilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedCollection.golden.ser"); } public void test_checkedListRandomAccessSerializationCompatability() throws Exception { List c = new ArrayList(); assertTrue(c instanceof RandomAccess); c = Collections.checkedList(c, String.class); - SerializationTester.assertCompabilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedListRandomAccess.golden.ser"); + SerializationTester.assertCompatibilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedListRandomAccess.golden.ser"); } public void test_checkedListSerializationCompatability() throws Exception { List c = new LinkedList(); assertFalse(c instanceof RandomAccess); c = Collections.checkedList(c, String.class); - SerializationTester.assertCompabilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedList.golden.ser"); + SerializationTester.assertCompatibilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedList.golden.ser"); } public void test_checkedSetSerializationCompatability() throws Exception { Set c = new HashSet(); assertFalse(c instanceof SortedSet); c = Collections.checkedSet(c, String.class); - SerializationTester.assertCompabilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedSet.golden.ser"); + SerializationTester.assertCompatibilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedSet.golden.ser"); } public void test_checkedMapSerializationCompatability() throws Exception { Map c = new HashMap(); assertFalse(c instanceof SortedMap); c = Collections.checkedMap(c, String.class, String.class); - SerializationTester.assertCompabilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedMap.golden.ser"); + SerializationTester.assertCompatibilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedMap.golden.ser"); } public void test_checkedSortedSetSerializationCompatability() throws Exception { SortedSet c = new TreeSet(); c = Collections.checkedSortedSet(c, String.class); - SerializationTester.assertCompabilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedSortedSet.golden.ser"); + SerializationTester.assertCompatibilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedSortedSet.golden.ser"); } public void test_checkedSortedMapSerializationCompatability() throws Exception { SortedMap c = new TreeMap(); c = Collections.checkedSortedMap(c, String.class, String.class); - SerializationTester.assertCompabilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedSortedMap.golden.ser"); + SerializationTester.assertCompatibilityEquals(c, "/serialization/org/apache/harmony/luni/tests/java/util/Collections_CheckedSortedMap.golden.ser"); } public void test_checkedCollectionLjava_util_CollectionLjava_lang_Class() { diff --git a/luni/src/test/java/libcore/java/util/OldPriorityQueueTest.java b/luni/src/test/java/libcore/java/util/OldPriorityQueueTest.java index d8750b5..c2e2a03 100644 --- a/luni/src/test/java/libcore/java/util/OldPriorityQueueTest.java +++ b/luni/src/test/java/libcore/java/util/OldPriorityQueueTest.java @@ -67,10 +67,8 @@ public class OldPriorityQueueTest extends TestCase { public void test_Serialization() throws Exception { Integer[] array = { 2, 45, 7, -12, 9, 23, 17, 1118, 10, 16, 39 }; List list = Arrays.asList(array); - PriorityQueue srcIntegerQueue = new PriorityQueue( - list); - PriorityQueue destIntegerQueue = (PriorityQueue) SerializationTester - .getDeserilizedObject(srcIntegerQueue); + PriorityQueue srcIntegerQueue = new PriorityQueue(list); + PriorityQueue destIntegerQueue = SerializationTester.getDeserializedObject(srcIntegerQueue); Arrays.sort(array); for (int i = 0; i < array.length; i++) { assertEquals(array[i], destIntegerQueue.poll()); @@ -81,10 +79,9 @@ public class OldPriorityQueueTest extends TestCase { public void test_Serialization_casting() throws Exception { Integer[] array = { 2, 45, 7, -12, 9, 23, 17, 1118, 10, 16, 39 }; List list = Arrays.asList(array); - PriorityQueue srcIntegerQueue = new PriorityQueue( - list); - PriorityQueue destStringQueue = (PriorityQueue) SerializationTester - .getDeserilizedObject(srcIntegerQueue); + PriorityQueue srcIntegerQueue = new PriorityQueue(list); + Object dodgy = SerializationTester.getDeserializedObject((Object) srcIntegerQueue); + PriorityQueue destStringQueue = (PriorityQueue) dodgy; // will not incur class cast exception. Object o = destStringQueue.peek(); Arrays.sort(array); -- cgit v1.1