6981157: Improve UnknownHostException with EAI error details and other cleanups
authormartin
Fri, 17 Sep 2010 14:40:38 -0700
changeset 6670 ae13809f3ce7
parent 6669 8f8d4d5768ae
child 6671 c5fbc05d7347
6981157: Improve UnknownHostException with EAI error details and other cleanups Summary: generify; remove compiler warnings, typos, casts; return status information via gai_strerror when getaddrinfo fails; show full stack of native failures Reviewed-by: michaelm, alanb
jdk/src/share/classes/java/net/InetAddress.java
jdk/src/solaris/native/java/net/Inet6AddressImpl.c
jdk/src/solaris/native/java/net/net_util_md.c
jdk/src/solaris/native/java/net/net_util_md.h
--- a/jdk/src/share/classes/java/net/InetAddress.java	Fri Sep 17 14:35:00 2010 -0700
+++ b/jdk/src/share/classes/java/net/InetAddress.java	Fri Sep 17 14:40:38 2010 -0700
@@ -677,19 +677,20 @@
 
     static InetAddressImpl  impl;
 
-    private static HashMap          lookupTable = new HashMap();
+    private static HashMap<String, InetAddress[]> lookupTable
+        = new HashMap<String, InetAddress[]>();
 
     /**
      * Represents a cache entry
      */
     static final class CacheEntry {
 
-        CacheEntry(Object address, long expiration) {
-            this.address = address;
+        CacheEntry(InetAddress[] addresses, long expiration) {
+            this.addresses = addresses;
             this.expiration = expiration;
         }
 
-        Object address;
+        InetAddress[] addresses;
         long expiration;
     }
 
@@ -698,7 +699,7 @@
      * at creation time.
      */
     static final class Cache {
-        private LinkedHashMap cache;
+        private LinkedHashMap<String, CacheEntry> cache;
         private Type type;
 
         enum Type {Positive, Negative};
@@ -708,7 +709,7 @@
          */
         public Cache(Type type) {
             this.type = type;
-            cache = new LinkedHashMap();
+            cache = new LinkedHashMap<String, CacheEntry>();
         }
 
         private int getPolicy() {
@@ -724,7 +725,7 @@
          * entry then for this host then the entry will be
          * replaced.
          */
-        public Cache put(String host, Object address) {
+        public Cache put(String host, InetAddress[] addresses) {
             int policy = getPolicy();
             if (policy == InetAddressCachePolicy.NEVER) {
                 return this;
@@ -736,12 +737,10 @@
 
                 // As we iterate in insertion order we can
                 // terminate when a non-expired entry is found.
-                LinkedList expired = new LinkedList();
-                Iterator i = cache.keySet().iterator();
+                LinkedList<String> expired = new LinkedList<String>();
                 long now = System.currentTimeMillis();
-                while (i.hasNext()) {
-                    String key = (String)i.next();
-                    CacheEntry entry = (CacheEntry)cache.get(key);
+                for (String key : cache.keySet()) {
+                    CacheEntry entry = cache.get(key);
 
                     if (entry.expiration >= 0 && entry.expiration < now) {
                         expired.add(key);
@@ -750,9 +749,8 @@
                     }
                 }
 
-                i = expired.iterator();
-                while (i.hasNext()) {
-                    cache.remove(i.next());
+                for (String key : expired) {
+                    cache.remove(key);
                 }
             }
 
@@ -766,7 +764,7 @@
             } else {
                 expiration = System.currentTimeMillis() + (policy * 1000);
             }
-            CacheEntry entry = new CacheEntry(address, expiration);
+            CacheEntry entry = new CacheEntry(addresses, expiration);
             cache.put(host, entry);
             return this;
         }
@@ -780,7 +778,7 @@
             if (policy == InetAddressCachePolicy.NEVER) {
                 return null;
             }
-            CacheEntry entry = (CacheEntry)cache.get(host);
+            CacheEntry entry = cache.get(host);
 
             // check if entry has expired
             if (entry != null && policy != InetAddressCachePolicy.FOREVER) {
@@ -814,42 +812,41 @@
     }
 
     /*
-     * Cache the given hostname and address.
+     * Cache the given hostname and addresses.
      */
-    private static void cacheAddress(String hostname, Object address,
-                                     boolean success) {
+    private static void cacheAddresses(String hostname,
+                                       InetAddress[] addresses,
+                                       boolean success) {
         hostname = hostname.toLowerCase();
         synchronized (addressCache) {
             cacheInitIfNeeded();
             if (success) {
-                addressCache.put(hostname, address);
+                addressCache.put(hostname, addresses);
             } else {
-                negativeCache.put(hostname, address);
+                negativeCache.put(hostname, addresses);
             }
         }
     }
 
     /*
      * Lookup hostname in cache (positive & negative cache). If
-     * found return address, null if not found.
+     * found return addresses, null if not found.
      */
-    private static Object getCachedAddress(String hostname) {
+    private static InetAddress[] getCachedAddresses(String hostname) {
         hostname = hostname.toLowerCase();
 
         // search both positive & negative caches
 
         synchronized (addressCache) {
-            CacheEntry entry;
-
             cacheInitIfNeeded();
 
-            entry = addressCache.get(hostname);
+            CacheEntry entry = addressCache.get(hostname);
             if (entry == null) {
                 entry = negativeCache.get(hostname);
             }
 
             if (entry != null) {
-                return entry.address;
+                return entry.addresses;
             }
         }
 
@@ -911,7 +908,7 @@
 
     static {
         // create the impl
-        impl = (new InetAddressImplFactory()).create();
+        impl = InetAddressImplFactory.create();
 
         // get name service if provided and requested
         String provider = null;;
@@ -931,7 +928,7 @@
         }
 
         // if not designate any name services provider,
-        // creat a default one
+        // create a default one
         if (nameServices.size() == 0) {
             NameService ns = createNSProvider("default");
             nameServices.add(ns);
@@ -939,7 +936,7 @@
     }
 
     /**
-     * Create an InetAddress based on the provided host name and IP address
+     * Creates an InetAddress based on the provided host name and IP address.
      * No name service is checked for the validity of the address.
      *
      * <p> The host name can either be a machine name, such as
@@ -1067,13 +1064,13 @@
 
         boolean ipv6Expected = false;
         if (host.charAt(0) == '[') {
-            // This is supposed to be an IPv6 litteral
+            // This is supposed to be an IPv6 literal
             if (host.length() > 2 && host.charAt(host.length()-1) == ']') {
                 host = host.substring(1, host.length() -1);
                 ipv6Expected = true;
             } else {
                 // This was supposed to be a IPv6 address, but it's not!
-                throw new UnknownHostException(host);
+                throw new UnknownHostException(host + ": invalid IPv6 address");
             }
         }
 
@@ -1180,8 +1177,6 @@
         throws UnknownHostException  {
         /* If it gets here it is presumed to be a hostname */
         /* Cache.get can return: null, unknownAddress, or InetAddress[] */
-        Object obj = null;
-        Object objcopy = null;
 
         /* make sure the connection to the host is allowed, before we
          * give out a hostname
@@ -1193,26 +1188,23 @@
             }
         }
 
-        obj = getCachedAddress(host);
+        InetAddress[] addresses = getCachedAddresses(host);
 
         /* If no entry in cache, then do the host lookup */
-        if (obj == null) {
-            obj = getAddressFromNameService(host);
+        if (addresses == null) {
+            addresses = getAddressesFromNameService(host);
         }
 
-        if (obj == unknown_array)
+        if (addresses == unknown_array)
             throw new UnknownHostException(host);
 
-        /* Make a copy of the InetAddress array */
-        objcopy = ((InetAddress [])obj).clone();
-
-        return (InetAddress [])objcopy;
+        return addresses.clone();
     }
 
-    private static Object getAddressFromNameService(String host)
+    private static InetAddress[] getAddressesFromNameService(String host)
         throws UnknownHostException
     {
-        Object obj = null;
+        InetAddress[] addresses = null;
         boolean success = false;
         UnknownHostException ex = null;
 
@@ -1226,16 +1218,16 @@
         //    would be blocked until the host is removed
         //    from the lookupTable. Then this thread
         //    should try to look up the addressCache.
-        //     i) if it found the address in the
+        //     i) if it found the addresses in the
         //        addressCache, checkLookupTable()  would
-        //        return the address.
-        //     ii) if it didn't find the address in the
+        //        return the addresses.
+        //     ii) if it didn't find the addresses in the
         //         addressCache for any reason,
         //         it should add the host in the
         //         lookupTable and return null so the
         //         following code would do  a lookup itself.
-        if ((obj = checkLookupTable(host)) == null) {
-            // This is the first thread which looks up the address
+        if ((addresses = checkLookupTable(host)) == null) {
+            // This is the first thread which looks up the addresses
             // this host or the cache entry for this host has been
             // expired so this thread should do the lookup.
             for (NameService nameService : nameServices) {
@@ -1246,26 +1238,26 @@
                      * allocating space when the lookup fails.
                      */
 
-                    obj = nameService.lookupAllHostAddr(host);
+                    addresses = nameService.lookupAllHostAddr(host);
                     success = true;
                     break;
                 } catch (UnknownHostException uhe) {
                     if (host.equalsIgnoreCase("localhost")) {
                         InetAddress[] local = new InetAddress[] { impl.loopbackAddress() };
-                        obj = local;
+                        addresses = local;
                         success = true;
                         break;
                     }
                     else {
-                        obj  = unknown_array;
+                        addresses = unknown_array;
                         success = false;
                         ex = uhe;
                     }
                 }
             }
 
-            // Cache the address.
-            cacheAddress(host, obj, success);
+            // Cache the addresses.
+            cacheAddresses(host, addresses, success);
             // Delete the host from the lookupTable, and
             // notify all threads waiting for the monitor
             // for lookupTable.
@@ -1274,13 +1266,13 @@
                 throw ex;
         }
 
-        return obj;
+        return addresses;
     }
 
 
-    private static Object checkLookupTable(String host) {
-        // make sure obj  is null.
-        Object obj = null;
+    private static InetAddress[] checkLookupTable(String host) {
+        // make sure addresses is null.
+        InetAddress[] addresses = null;
 
         synchronized (lookupTable) {
             // If the host isn't in the lookupTable, add it in the
@@ -1288,11 +1280,11 @@
             // the lookup.
             if (lookupTable.containsKey(host) == false) {
                 lookupTable.put(host, null);
-                return obj;
+                return addresses;
             }
 
             // If the host is in the lookupTable, it means that another
-            // thread is trying to look up the address of this host.
+            // thread is trying to look up the addresses of this host.
             // This thread should wait.
             while (lookupTable.containsKey(host)) {
                 try {
@@ -1302,18 +1294,18 @@
             }
         }
 
-        // The other thread has finished looking up the address of
-        // the host. This thread should retry to get the address
-        // from the addressCache. If it doesn't get the address from
-        // the cache,  it will try to look up the address itself.
-        obj = getCachedAddress(host);
-        if (obj == null) {
+        // The other thread has finished looking up the addresses of
+        // the host. This thread should retry to get the addresses
+        // from the addressCache. If it doesn't get the addresses from
+        // the cache, it will try to look up the addresses itself.
+        addresses = getCachedAddresses(host);
+        if (addresses == null) {
             synchronized (lookupTable) {
                 lookupTable.put(host, null);
             }
         }
 
-        return obj;
+        return addresses;
     }
 
     private static void updateLookupTable(String host) {
@@ -1396,15 +1388,20 @@
                         cachedLocalHost = null;
                 }
 
-                // we are calling getAddressFromNameService directly
+                // we are calling getAddressesFromNameService directly
                 // to avoid getting localHost from cache
                 if (ret == null) {
                     InetAddress[] localAddrs;
                     try {
                         localAddrs =
-                            (InetAddress[]) InetAddress.getAddressFromNameService(local);
+                            InetAddress.getAddressesFromNameService(local);
                     } catch (UnknownHostException uhe) {
-                        throw new UnknownHostException(local + ": " + uhe.getMessage());
+                        // Rethrow with a more informative error message.
+                        UnknownHostException uhe2 =
+                            new UnknownHostException(local + ": " +
+                                                     uhe.getMessage());
+                        uhe2.initCause(uhe);
+                        throw uhe2;
                     }
                     cachedLocalHost = localAddrs[0];
                     cacheTime = now;
@@ -1434,8 +1431,8 @@
     /*
      * Load and instantiate an underlying impl class
      */
-    static Object loadImpl(String implName) {
-        Object impl;
+    static InetAddressImpl loadImpl(String implName) {
+        Object impl = null;
 
         /*
          * Property "impl.prefix" will be prepended to the classname
@@ -1446,7 +1443,6 @@
          */
         String prefix = AccessController.doPrivileged(
                       new GetPropertyAction("impl.prefix", ""));
-        impl = null;
         try {
             impl = Class.forName("java.net." + prefix + implName).newInstance();
         } catch (ClassNotFoundException e) {
@@ -1471,7 +1467,7 @@
             }
         }
 
-        return impl;
+        return (InetAddressImpl) impl;
     }
 
     private void readObjectNoData (ObjectInputStream s) throws
@@ -1498,13 +1494,8 @@
 class InetAddressImplFactory {
 
     static InetAddressImpl create() {
-        Object o;
-        if (isIPv6Supported()) {
-            o = InetAddress.loadImpl("Inet6AddressImpl");
-        } else {
-            o = InetAddress.loadImpl("Inet4AddressImpl");
-        }
-        return (InetAddressImpl)o;
+        return InetAddress.loadImpl(isIPv6Supported() ?
+                                    "Inet6AddressImpl" : "Inet4AddressImpl");
     }
 
     static native boolean isIPv6Supported();
--- a/jdk/src/solaris/native/java/net/Inet6AddressImpl.c	Fri Sep 17 14:35:00 2010 -0700
+++ b/jdk/src/solaris/native/java/net/Inet6AddressImpl.c	Fri Sep 17 14:40:38 2010 -0700
@@ -124,7 +124,7 @@
 static int initialized = 0;
 
 /*
- * Find an internet address for a given hostname.  Not this this
+ * Find an internet address for a given hostname.  Note that this
  * code only works for addresses of type INET. The translation
  * of %d.%d.%d.%d to an address (int) occurs in java now, so the
  * String "host" shouldn't *ever* be a %d.%d.%d.%d string
@@ -200,7 +200,7 @@
          */
         if (isspace((unsigned char)hostname[0])) {
             JNU_ThrowByName(env, JNU_JAVANETPKG "UnknownHostException",
-                            (char *)hostname);
+                            hostname);
             JNU_ReleaseStringPlatformChars(env, host, hostname);
             return NULL;
         }
@@ -210,8 +210,7 @@
 
         if (error) {
             /* report error */
-            JNU_ThrowByName(env, JNU_JAVANETPKG "UnknownHostException",
-                            (char *)hostname);
+            ThrowUnknownHostExceptionWithGaiError(env, hostname, error);
             JNU_ReleaseStringPlatformChars(env, host, hostname);
             return NULL;
         } else {
@@ -407,7 +406,7 @@
             addr |= ((caddr[1] <<16) & 0xff0000);
             addr |= ((caddr[2] <<8) & 0xff00);
             addr |= (caddr[3] & 0xff);
-            memset((char *) &him4, 0, sizeof(him4));
+            memset((void *) &him4, 0, sizeof(him4));
             him4.sin_addr.s_addr = (uint32_t) htonl(addr);
             him4.sin_family = AF_INET;
             sa = (struct sockaddr *) &him4;
@@ -417,7 +416,7 @@
              * For IPv6 address construct a sockaddr_in6 structure.
              */
             (*env)->GetByteArrayRegion(env, addrArray, 0, 16, caddr);
-            memset((char *) &him6, 0, sizeof(him6));
+            memset((void *) &him6, 0, sizeof(him6));
             memcpy((void *)&(him6.sin6_addr), caddr, sizeof(struct in6_addr) );
             him6.sin6_family = AF_INET6;
             sa = (struct sockaddr *) &him6 ;
@@ -579,8 +578,8 @@
                                                          ifArray, ttl);
     }
 
-    memset((char *) caddr, 0, 16);
-    memset((char *) &him6, 0, sizeof(him6));
+    memset((void *) caddr, 0, 16);
+    memset((void *) &him6, 0, sizeof(him6));
     (*env)->GetByteArrayRegion(env, addrArray, 0, 16, caddr);
     memcpy((void *)&(him6.sin6_addr), caddr, sizeof(struct in6_addr) );
     him6.sin6_family = AF_INET6;
@@ -600,8 +599,8 @@
      * for it.
      */
     if (!(IS_NULL(ifArray))) {
-      memset((char *) caddr, 0, 16);
-      memset((char *) &inf6, 0, sizeof(inf6));
+      memset((void *) caddr, 0, 16);
+      memset((void *) &inf6, 0, sizeof(inf6));
       (*env)->GetByteArrayRegion(env, ifArray, 0, 16, caddr);
       memcpy((void *)&(inf6.sin6_addr), caddr, sizeof(struct in6_addr) );
       inf6.sin6_family = AF_INET6;
--- a/jdk/src/solaris/native/java/net/net_util_md.c	Fri Sep 17 14:35:00 2010 -0700
+++ b/jdk/src/solaris/native/java/net/net_util_md.c	Fri Sep 17 14:40:38 2010 -0700
@@ -61,6 +61,7 @@
 
 getaddrinfo_f getaddrinfo_ptr = NULL;
 freeaddrinfo_f freeaddrinfo_ptr = NULL;
+gai_strerror_f gai_strerror_ptr = NULL;
 getnameinfo_f getnameinfo_ptr = NULL;
 
 /*
@@ -342,11 +343,14 @@
     freeaddrinfo_ptr = (freeaddrinfo_f)
         JVM_FindLibraryEntry(RTLD_DEFAULT, "freeaddrinfo");
 
+    gai_strerror_ptr = (gai_strerror_f)
+        JVM_FindLibraryEntry(RTLD_DEFAULT, "gai_strerror");
+
     getnameinfo_ptr = (getnameinfo_f)
         JVM_FindLibraryEntry(RTLD_DEFAULT, "getnameinfo");
 
     if (freeaddrinfo_ptr == NULL || getnameinfo_ptr == NULL) {
-        /* Wee need all 3 of them */
+        /* We need all 3 of them */
         getaddrinfo_ptr = NULL;
     }
 
@@ -355,6 +359,32 @@
 #endif /* AF_INET6 */
 }
 
+void ThrowUnknownHostExceptionWithGaiError(JNIEnv *env,
+                                           const char* hostname,
+                                           int gai_error)
+{
+    const char *format = "%s: %s";
+    const char *error_string =
+        (gai_strerror_ptr == NULL) ? NULL : (*gai_strerror_ptr)(gai_error);
+    if (error_string == NULL)
+        error_string = "unknown error";
+
+    int size = strlen(format) + strlen(hostname) + strlen(error_string) + 2;
+    char *buf = (char *) malloc(size);
+    if (buf) {
+        sprintf(buf, format, hostname, error_string);
+        jstring s = JNU_NewStringPlatform(env, buf);
+        if (s != NULL) {
+            jobject x = JNU_NewObjectByName(env,
+                                            "java/net/UnknownHostException",
+                                            "(Ljava/lang/String;)V", s);
+            if (x != NULL)
+                (*env)->Throw(env, x);
+        }
+        free(buf);
+    }
+}
+
 void
 NET_AllocSockaddr(struct sockaddr **him, int *len) {
 #ifdef AF_INET6
--- a/jdk/src/solaris/native/java/net/net_util_md.h	Fri Sep 17 14:35:00 2010 -0700
+++ b/jdk/src/solaris/native/java/net/net_util_md.h	Fri Sep 17 14:40:38 2010 -0700
@@ -84,11 +84,13 @@
 
 /* needed from libsocket on Solaris 8 */
 
-typedef int (*getaddrinfo_f)(const char *nodename, const char  *servname,
-     const struct addrinfo *hints, struct addrinfo **res);
+typedef int (*getaddrinfo_f)(const char *nodename, const char *servname,
+    const struct addrinfo *hints, struct addrinfo **res);
 
 typedef void (*freeaddrinfo_f)(struct addrinfo *);
 
+typedef const char * (*gai_strerror_f)(int ecode);
+
 typedef int (*getnameinfo_f)(const struct sockaddr *, size_t,
     char *, size_t, char *, size_t, int);
 
@@ -96,6 +98,10 @@
 extern freeaddrinfo_f freeaddrinfo_ptr;
 extern getnameinfo_f getnameinfo_ptr;
 
+void ThrowUnknownHostExceptionWithGaiError(JNIEnv *env,
+                                           const char* hostname,
+                                           int gai_error);
+
 /* do we have address translation support */
 
 extern jboolean NET_addrtransAvailable();