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

5 files changed, 68 insertions, 122 deletions

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 <i>(ICMP ECHO REQUEST)</i>. When first step fails, a TCP connection
-     * on port 7 (Echo) of the remote host is established.
+     * ICMP <i>(ICMP ECHO REQUEST)</i>, 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 <i>(ICMP ECHO REQUEST)</i>. When first step fails, a TCP connection
-     * on port 7 (Echo) of the remote host is established.
+     * ICMP <i>(ICMP ECHO REQUEST)</i>, 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<InetAddress> addresses = Collections.list(netif.getInetAddresses());
-        if (addresses.isEmpty()) {
+        // Try each NetworkInterface in parallel.
+        // Use a thread pool Executor?
+        List<InetAddress> 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<NetworkInterface> NetworkInterfaces = NetworkInterface
-                        .getNetworkInterfaces();
-                while (NetworkInterfaces.hasMoreElements()) {
-                    NetworkInterface networkInterface = NetworkInterfaces
-                            .nextElement();
-                    Enumeration<InetAddress> 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<String> 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<String> c = new ArrayList<String>();
         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<String> c = new LinkedList<String>();
         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<String> c = new HashSet<String>();
         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<String, String> c = new HashMap<String, String>();
         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<String> c = new TreeSet<String>();
         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<String, String> c = new TreeMap<String, String>();
         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<Integer> list = Arrays.asList(array);
-        PriorityQueue<Integer> srcIntegerQueue = new PriorityQueue<Integer>(
-                list);
-        PriorityQueue<Integer> destIntegerQueue = (PriorityQueue<Integer>) SerializationTester
-                .getDeserilizedObject(srcIntegerQueue);
+        PriorityQueue<Integer> srcIntegerQueue = new PriorityQueue<Integer>(list);
+        PriorityQueue<Integer> 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<Integer> list = Arrays.asList(array);
-        PriorityQueue<Integer> srcIntegerQueue = new PriorityQueue<Integer>(
-                list);
-        PriorityQueue<String> destStringQueue = (PriorityQueue<String>) SerializationTester
-                .getDeserilizedObject(srcIntegerQueue);
+        PriorityQueue<Integer> srcIntegerQueue = new PriorityQueue<Integer>(list);
+        Object dodgy = SerializationTester.getDeserializedObject((Object) srcIntegerQueue);
+        PriorityQueue<String> destStringQueue = (PriorityQueue<String>) dodgy;
         // will not incur class cast exception.
         Object o = destStringQueue.peek();
         Arrays.sort(array);