Ingmar Runge provided a source snippet that caused a crash. The reason for

the crash was that libcurl internally was a bit confused about who owned the
DNS cache at all times so if you created an easy handle that uses a shared
DNS cache and added that to a multi handle it would crash. Now we keep more
careful internal track of exactly what kind of DNS cache each easy handle
uses: None, Private (allocated for and used only by this single handle),
Shared (points to a cache held by a shared object), Global (points to the
global cache) or Multi (points to the cache within the multi handle that is
automatically shared between all easy handles that are added with private
caches).

diff --git a/CHANGES b/CHANGES
index e1d6b5687cf965085c138b5de3a48cd25bd52526..574d513b5758d4cb58c2f8d2dbe79b97ec9456a8 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,18 @@
 
                                   Changelog
 
+Daniel (8 July 2006)
+- Ingmar Runge provided a source snippet that caused a crash. The reason for
+  the crash was that libcurl internally was a bit confused about who owned the
+  DNS cache at all times so if you created an easy handle that uses a shared
+  DNS cache and added that to a multi handle it would crash. Now we keep more
+  careful internal track of exactly what kind of DNS cache each easy handle
+  uses: None, Private (allocated for and used only by this single handle),
+  Shared (points to a cache held by a shared object), Global (points to the
+  global cache) or Multi (points to the cache within the multi handle that is
+  automatically shared between all easy handles that are added with private
+  caches).
+
 Daniel (4 July 2006)
 - Toshiyuki Maezawa fixed a problem where you couldn't override the
   Proxy-Connection: header when using a proxy and not doing CONNECT.

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 06c7d5a34437500546c7a6a30a5b639a7c1cc16f..d8c66cdd2f03075c9c9347b6a50ac9c6c9b004d7 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -18,6 +18,7 @@ This release includes the following changes:
 
 This release includes the following bugfixes:
 
+ o an easy handle with shared DNS cache added to a multi handle caused a crash
  o couldn't override the Proxy-Connection: header for non-CONNECT requests
  o curl_multi_fdset() could wrongly return -1 as max_fd value
 
@@ -35,6 +36,7 @@ New curl mirrors:
 This release would not have looked like this without help, code, reports and
 advice from friends like these:
 
- Dan Fandrich, Peter Silva, Arve Knudsen, Michael Wallner, Toshiyuki Maezawa
+ Dan Fandrich, Peter Silva, Arve Knudsen, Michael Wallner, Toshiyuki Maezawa,
+ Ingmar Runge
 
         Thanks! (and sorry if I forgot to mention someone)

diff --git a/lib/easy.c b/lib/easy.c
index fdbb6a0e26fb031c57cd7d5016ee1db49f9b0c24..2784db83f0104f697f5b2806a598fd0aa9f24218 100644 (file)
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2005, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2006, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -436,16 +436,18 @@ CURLcode curl_easy_perform(CURL *curl)
   if ( ! (data->share && data->share->hostcache) ) {
 
     if (Curl_global_host_cache_use(data) &&
-        data->hostcache != Curl_global_host_cache_get()) {
-      if (data->hostcache)
-        Curl_hash_destroy(data->hostcache);
-      data->hostcache = Curl_global_host_cache_get();
+        (data->dns.hostcachetype != HCACHE_GLOBAL)) {
+      if (data->dns.hostcachetype == HCACHE_PRIVATE)
+        Curl_hash_destroy(data->dns.hostcache);
+      data->dns.hostcache = Curl_global_host_cache_get();
+      data->dns.hostcachetype = HCACHE_GLOBAL;
     }
 
-    if (!data->hostcache) {
-      data->hostcache = Curl_mk_dnscache();
+    if (!data->dns.hostcache) {
+      data->dns.hostcachetype = HCACHE_PRIVATE;
+      data->dns.hostcache = Curl_mk_dnscache();
 
-      if(!data->hostcache)
+      if(!data->dns.hostcache)
         /* While we possibly could survive and do good without a host cache,
            the fact that creating it failed indicates that things are truly
            screwed up and we should bail out! */

diff --git a/lib/hostip.c b/lib/hostip.c
index ca08524ea9affa281af0790386b50245dd9c2398..7f071efb20f866f15499c30c5945b6188af584bc 100644 (file)
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -255,7 +255,7 @@ void Curl_hostcache_prune(struct SessionHandle *data)
 {
   time_t now;
 
-  if((data->set.dns_cache_timeout == -1) || !data->hostcache)
+  if((data->set.dns_cache_timeout == -1) || !data->dns.hostcache)
     /* cache forever means never prune, and NULL hostcache means
        we can't do it */
     return;
@@ -266,7 +266,7 @@ void Curl_hostcache_prune(struct SessionHandle *data)
   time(&now);
 
   /* Remove outdated and unused entries from the hostcache */
-  hostcache_prune(data->hostcache,
+  hostcache_prune(data->dns.hostcache,
                   data->set.dns_cache_timeout,
                   now);
 
@@ -279,7 +279,7 @@ remove_entry_if_stale(struct SessionHandle *data, struct Curl_dns_entry *dns)
 {
   struct hostcache_prune_data user;
 
-  if( !dns || (data->set.dns_cache_timeout == -1) || !data->hostcache)
+  if( !dns || (data->set.dns_cache_timeout == -1) || !data->dns.hostcache)
     /* cache forever means never prune, and NULL hostcache means
        we can't do it */
     return 0;
@@ -296,7 +296,7 @@ remove_entry_if_stale(struct SessionHandle *data, struct Curl_dns_entry *dns)
   if(data->share)
     Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE);
 
-  Curl_hash_clean_with_criterium(data->hostcache,
+  Curl_hash_clean_with_criterium(data->dns.hostcache,
                                  (void *) &user,
                                  hostcache_timestamp_remove);
 
@@ -356,7 +356,8 @@ Curl_cache_addr(struct SessionHandle *data,
   /* Store the resolved data in our DNS cache. This function may return a
      pointer to an existing struct already present in the hash, and it may
      return the same argument we pass in. Make no assumptions. */
-  dns2 = Curl_hash_add(data->hostcache, entry_id, entry_len+1, (void *)dns);
+  dns2 = Curl_hash_add(data->dns.hostcache, entry_id, entry_len+1,
+                       (void *)dns);
   if(!dns2) {
     /* Major badness, run away. */
     free(dns);
@@ -428,7 +429,7 @@ int Curl_resolv(struct connectdata *conn,
     Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE);
 
   /* See if its already in our dns cache */
-  dns = Curl_hash_pick(data->hostcache, entry_id, entry_len+1);
+  dns = Curl_hash_pick(data->dns.hostcache, entry_id, entry_len+1);
 
   if(data->share)
     Curl_share_unlock(data, CURL_LOCK_DATA_DNS);

diff --git a/lib/multi.c b/lib/multi.c
index 9aee31ddd557a2ae63a11fa3f8f04f85e43e6a75..5f98c2eaf42296d03364940aa4f9b6081fdaa304 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -317,13 +317,14 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle,
   easy->easy_handle = easy_handle;
   multistate(easy, CURLM_STATE_INIT);
 
-  /* for multi interface connections, we share DNS cache automaticly.
-     First kill the existing one if there is any. */
-  if (easy->easy_handle->hostcache &&
-      easy->easy_handle->hostcache != multi->hostcache)
-    Curl_hash_destroy(easy->easy_handle->hostcache);
-
-  easy->easy_handle->hostcache = multi->hostcache;
+  /* for multi interface connections, we share DNS cache automaticly if the
+     easy handle's one is currently private. */
+  if (easy->easy_handle->dns.hostcache &&
+      (easy->easy_handle->dns.hostcachetype == HCACHE_PRIVATE)) {
+    Curl_hash_destroy(easy->easy_handle->dns.hostcache);
+    easy->easy_handle->dns.hostcache = multi->hostcache;
+    easy->easy_handle->dns.hostcachetype = HCACHE_MULTI;
+  }
 
   /* We add this new entry first in the list. We make our 'next' point to the
      previous next and our 'prev' point back to the 'first' struct */
@@ -374,8 +375,12 @@ CURLMcode curl_multi_remove_handle(CURLM *multi_handle,
     /* If the 'state' is not INIT or COMPLETED, we might need to do something
        nice to put the easy_handle in a good known state when this returns. */
 
-    /* clear out the usage of the shared DNS cache */
-    easy->easy_handle->hostcache = NULL;
+    if(easy->easy_handle->dns.hostcachetype == HCACHE_MULTI) {
+      /* clear out the usage of the shared DNS cache */
+      easy->easy_handle->dns.hostcache = NULL;
+      easy->easy_handle->dns.hostcachetype = HCACHE_NONE;
+    }
+
     Curl_easy_addmulti(easy->easy_handle, NULL); /* clear the association
                                                     to this multi handle */
 
@@ -893,8 +898,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
   } while (easy->easy_handle->change.url_changed);
 
   if ((CURLM_STATE_COMPLETED == easy->state) && !easy->msg) {
-    /* clear out the usage of the shared DNS cache */
-    easy->easy_handle->hostcache = NULL;
+    if(easy->easy_handle->dns.hostcachetype == HCACHE_MULTI) {
+      /* clear out the usage of the shared DNS cache */
+      easy->easy_handle->dns.hostcache = NULL;
+      easy->easy_handle->dns.hostcachetype = HCACHE_NONE;
+    }
 
     /* now add a node to the Curl_message linked list with this info */
     msg = (struct Curl_message *)malloc(sizeof(struct Curl_message));
@@ -975,8 +983,11 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle)
     easy = multi->easy.next;
     while(easy) {
       nexteasy=easy->next;
-      /* clear out the usage of the shared DNS cache */
-      easy->easy_handle->hostcache = NULL;
+      if(easy->easy_handle->dns.hostcachetype == HCACHE_MULTI) {
+        /* clear out the usage of the shared DNS cache */
+        easy->easy_handle->dns.hostcache = NULL;
+        easy->easy_handle->dns.hostcachetype = HCACHE_NONE;
+      }
       Curl_easy_addmulti(easy->easy_handle, NULL); /* clear the association */
 
       if (easy->msg)

diff --git a/lib/url.c b/lib/url.c
index 85537b2ce53324e22b540b2e754fdc442107d5a9..6e50e7a031ce956449cfbbef6c201c79dafc7e1d 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -205,7 +205,7 @@ CURLcode Curl_close(struct SessionHandle *data)
 
   if ( ! (data->share && data->share->hostcache) ) {
     if ( !Curl_global_host_cache_use(data)) {
-      Curl_hash_destroy(data->hostcache);
+      Curl_hash_destroy(data->dns.hostcache);
     }
   }
 
@@ -1392,8 +1392,10 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option,
       if(data->share) {
         Curl_share_lock(data, CURL_LOCK_DATA_SHARE, CURL_LOCK_ACCESS_SINGLE);
 
-        if(data->share->hostcache == data->hostcache)
-          data->hostcache = NULL;
+        if(data->dns.hostcachetype == HCACHE_SHARED) {
+          data->dns.hostcache = NULL;
+          data->dns.hostcachetype = HCACHE_NONE;
+        }
 
         if(data->share->cookies == data->cookies)
           data->cookies = NULL;
@@ -1413,11 +1415,12 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option,
         data->share->dirty++;
 
         if(data->share->hostcache) {
-          /* use shared host cache, first free own one if any */
-          if(data->hostcache)
-            Curl_hash_destroy(data->hostcache);
+          /* use shared host cache, first free the private one if any */
+          if(data->dns.hostcachetype == HCACHE_PRIVATE)
+            Curl_hash_destroy(data->dns.hostcache);
 
-          data->hostcache = data->share->hostcache;
+          data->dns.hostcache = data->share->hostcache;
+          data->dns.hostcachetype = HCACHE_SHARED;
         }
 #if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES)
         if(data->share->cookies) {

diff --git a/lib/urldata.h b/lib/urldata.h
index 834741fcc2c86e759b8bf9b3c80da56ba3c957a8..63ccfe70aecfd19dd321b44df22a178149c438b0 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1145,19 +1145,29 @@ struct UserDefined {
   bool connect_only;     /* make connection, let application use the socket */
 };
 
+struct Names {
+  struct curl_hash *hostcache;
+  enum {
+    HCACHE_NONE,    /* not pointing to anything */
+    HCACHE_PRIVATE, /* points to our own */
+    HCACHE_GLOBAL,  /* points to the (shrug) global one */
+    HCACHE_MULTI,   /* points to a shared one in the multi handle */
+    HCACHE_SHARED   /* points to a shared one in a shared object */
+  } hostcachetype;
+};
+
 /*
- * In August 2001, this struct was redesigned and is since stricter than
- * before. The 'connectdata' struct MUST have all the connection oriented
- * stuff as we may now have several simultaneous connections and connection
- * structs in memory.
+ * The 'connectdata' struct MUST have all the connection oriented stuff as we
+ * may have several simultaneous connections and connection structs in memory.
  *
- * From now on, the 'SessionHandle' must only contain data that is set once to
- * go for many (perhaps) independent connections. Values that are generated or
+ * The 'struct UserDefined' must only contain data that is set once to go for
+ * many (perhaps) independent connections. Values that are generated or
  * calculated internally for the "session handle" must be defined within the
- * 'struct UrlState' instead.  */
+ * 'struct UrlState' instead.
+ */
 
 struct SessionHandle {
-  struct curl_hash *hostcache;
+  struct Names dns;
   struct Curl_multi *multi;    /* if non-NULL, points to the multi handle
                                   struct of which this "belongs" */
   struct Curl_share *share;    /* Share, handles global variable mutexing */