When transferring 500 downloads in parallel with a c-ares enabled build only

to find that it crashed miserably, and this was due to some select()isms left
in the code. This was due to API restrictions in c-ares 1.3.x, but with the
upcoming c-ares 1.4.0 this is no longer the case so now libcurl runs much
better with c-ares and the multi interface with > 1024 file descriptors in
use.

8 files changed, 92 insertions, 263 deletions

diff --git a/CHANGES b/CHANGES
index 764210e37..405384d0c 100644
--- a/CHANGES
+++ b/CHANGES
@@ -10,6 +10,16 @@ Daniel S (31 May 2007)
 - Feng Tu made (lib)curl support "upload" resuming work for file:// URLs.
 
 Daniel S (30 May 2007)
+- I modified the 10-at-a-time.c example to transfer 500 downloads in parallel
+  with a c-ares enabled build only to find that it crashed miserably, and this
+  was due to some select()isms left in the code. This was due to API
+  restrictions in c-ares 1.3.x, but with the upcoming c-ares 1.4.0 this is no
+  longer the case so now libcurl runs much better with c-ares and the multi
+  interface with > 1024 file descriptors in use.
+
+  Extra note: starting now we require c-ares 1.4.0 for asynchronous name
+  resolves.
+
 - Added CURLMOPT_MAXCONNECTS which is a curl_multi_setopt() option for setting
   the maximum size of the connection cache maximum size of the multi handle.
 
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 591034fce..0acb94558 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -19,6 +19,7 @@ This release includes the following changes:
  o SFTP now supports quote commands before a transfer
  o CURLMOPT_MAXCONNECTS added to curl_multi_setopt()
  o upload resume works for file:// URLs
+ o asynchronous name resolves now require c-ares 1.4.0 or later
 
 This release includes the following bugfixes:
 
diff --git a/configure.ac b/configure.ac
index 76b9dc86c..01b072625 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2050,10 +2050,10 @@ fi
 dnl set variable for use in automakefile(s)
 AM_CONDITIONAL(USE_MANUAL, test x"$USE_MANUAL" = x1)
 
-AC_MSG_CHECKING([whether to enable ares])
+AC_MSG_CHECKING([whether to enable c-ares])
 AC_ARG_ENABLE(ares,
-AC_HELP_STRING([--enable-ares=PATH],[Enable ares for name lookups])
-AC_HELP_STRING([--disable-ares],[Disable ares for name lookups]),
+AC_HELP_STRING([--enable-ares=PATH],[Enable c-ares for name lookups])
+AC_HELP_STRING([--disable-ares],[Disable c-ares for name lookups]),
 [ case "$enableval" in
   no)
        AC_MSG_RESULT(no)
@@ -2061,10 +2061,10 @@ AC_HELP_STRING([--disable-ares],[Disable ares for name lookups]),
   *)   AC_MSG_RESULT(yes)
 
        if test "x$IPV6_ENABLED" = "x1"; then
-         AC_MSG_NOTICE([ares may not work properly with ipv6])
+         AC_MSG_NOTICE([c-ares may not work properly with ipv6])
        fi
 
-       AC_DEFINE(USE_ARES, 1, [Define if you want to enable ares support])
+       AC_DEFINE(USE_ARES, 1, [Define if you want to enable c-ares support])
        dnl substitute HAVE_ARES for curl-config and similar
        HAVE_ARES="1"
        AC_SUBST(HAVE_ARES)
@@ -2109,7 +2109,8 @@ void curl_domalloc() { }
 int main(void)
 {
   ares_channel channel;
-  ares_cancel(channel);
+  ares_cancel(channel); /* added in 1.2.0 */
+  ares_process_fd(channel, 0, 0); /* added in 1.4.0 */
   return 0;
 }
 ],
diff --git a/lib/README.ares b/lib/README.ares
index b41b70924..b94a16611 100644
--- a/lib/README.ares
+++ b/lib/README.ares
@@ -12,8 +12,7 @@ c-ares:
   http://daniel.haxx.se/projects/c-ares/
 
 NOTE
-  The latest libcurl version requires c-ares 1.3.2 or later to work
-  flawlessly.
+  The latest libcurl version requires c-ares 1.4.0 or later.
 
   Once upon the time libcurl built fine with the "original" ares. That is no
   longer true. You need to use c-ares.
diff --git a/lib/easy.c b/lib/easy.c
index 8f590c97b..d5816bc48 100644
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -420,11 +420,14 @@ CURLcode curl_easy_perform(CURL *easy)
     timeout.tv_sec = 1;
     timeout.tv_usec = 0;
 
-    /* get file descriptors from the transfers */
+    /* Old deprecated style: get file descriptors from the transfers */
     curl_multi_fdset(multi, &fdread, &fdwrite, &fdexcep, &maxfd);
-
     rc = Curl_select(maxfd+1, &fdread, &fdwrite, &fdexcep, &timeout);
 
+    /* The way is to extract the sockets and wait for them without using
+       select. This whole alternative version should probably rather use the
+       curl_multi_socket() approach. */
+
     if(rc == -1)
       /* select error */
       break;
diff --git a/lib/hostares.c b/lib/hostares.c
index dae1ca3b7..3aa7782bb 100644
--- a/lib/hostares.c
+++ b/lib/hostares.c
@@ -126,6 +126,69 @@ int Curl_resolv_getsock(struct connectdata *conn,
 }
 
 /*
+ * ares_waitperform()
+ *
+ * 1) Ask ares what sockets it currently plays with, then
+ * 2) wait for the timeout period to check for action on ares' sockets.
+ * 3) tell ares to act on all the sockets marked as "with action"
+ *
+ * return number of sockets it worked on
+ */
+
+static int ares_waitperform(struct connectdata *conn, int timeout_ms)
+{
+  struct SessionHandle *data = conn->data;
+  int nfds;
+  int bitmask;
+  int socks[ARES_GETSOCK_MAXNUM];
+  struct pollfd pfd[ARES_GETSOCK_MAXNUM];
+  int m;
+  int i;
+  int num;
+
+  bitmask = ares_getsock(data->state.areschannel, socks, ARES_GETSOCK_MAXNUM);
+
+  for(i=0; i < ARES_GETSOCK_MAXNUM; i++) {
+    pfd[i].events = 0;
+    m=0;
+    if(ARES_GETSOCK_READABLE(bitmask, i)) {
+      pfd[i].fd = socks[i];
+      pfd[i].events |= POLLRDNORM|POLLIN;
+      m=1;
+    }
+    if(ARES_GETSOCK_WRITABLE(bitmask, i)) {
+      pfd[i].fd = socks[i];
+      pfd[i].events |= POLLWRNORM|POLLOUT;
+      m=1;
+    }
+    pfd[i].revents=0;
+    if(!m)
+      break;
+  }
+  num = i;
+
+  if(num)
+    nfds = Curl_poll(pfd, num, timeout_ms);
+  else
+    nfds = 0;
+
+  if(!nfds)
+    /* Call ares_process() unconditonally here, even if we simply timed out
+       above, as otherwise the ares name resolve won't timeout! */
+    ares_process_fd(data->state.areschannel, ARES_SOCKET_BAD, ARES_SOCKET_BAD);
+  else {
+    /* move through the descriptors and ask for processing on them */
+    for(i=0; i < num; i++)
+      ares_process_fd(data->state.areschannel,
+                      pfd[i].revents & (POLLRDNORM|POLLIN)?
+                      pfd[i].fd:ARES_SOCKET_BAD,
+                      pfd[i].revents & (POLLWRNORM|POLLOUT)?
+                      pfd[i].fd:ARES_SOCKET_BAD);
+  }
+  return nfds;
+}
+
+/*
  * Curl_is_resolved() is called repeatedly to check if a previous name resolve
  * request has completed. It should also make sure to time-out if the
  * operation seems to take too long.
@@ -135,25 +198,12 @@ int Curl_resolv_getsock(struct connectdata *conn,
 CURLcode Curl_is_resolved(struct connectdata *conn,
                           struct Curl_dns_entry **dns)
 {
-  fd_set read_fds, write_fds;
-  struct timeval tv={0,0};
   struct SessionHandle *data = conn->data;
-  int nfds;
-
-  FD_ZERO(&read_fds);
-  FD_ZERO(&write_fds);
-
-  nfds = ares_fds(data->state.areschannel, &read_fds, &write_fds);
-
-  (void)Curl_select(nfds, &read_fds, &write_fds, NULL,
-                    (struct timeval *)&tv);
-
-  /* Call ares_process() unconditonally here, even if we simply timed out
-     above, as otherwise the ares name resolve won't timeout! */
-  ares_process(data->state.areschannel, &read_fds, &write_fds);
 
   *dns = NULL;
 
+  ares_waitperform(conn, 0);
+
   if(conn->async.done) {
     /* we're done, kill the ares handle */
     if(!conn->async.dns) {
@@ -194,28 +244,18 @@ CURLcode Curl_wait_for_resolv(struct connectdata *conn,
 
   /* Wait for the name resolve query to complete. */
   while (1) {
-    int nfds=0;
-    fd_set read_fds, write_fds;
     struct timeval *tvp, tv, store;
-    int count;
     struct timeval now = Curl_tvnow();
     long timediff;
 
     store.tv_sec = (int)timeout/1000;
     store.tv_usec = (timeout%1000)*1000;
 
-    FD_ZERO(&read_fds);
-    FD_ZERO(&write_fds);
-    nfds = ares_fds(data->state.areschannel, &read_fds, &write_fds);
-    if (nfds == 0)
-      /* no file descriptors means we're done waiting */
-      break;
     tvp = ares_timeout(data->state.areschannel, &store, &tv);
-    count = Curl_select(nfds, &read_fds, &write_fds, NULL, tvp);
-    if ((count < 0) && (SOCKERRNO != EINVAL))
-      break;
 
-    ares_process(data->state.areschannel, &read_fds, &write_fds);
+    if(!ares_waitperform(conn, tv.tv_sec * 1000 + tv.tv_usec/1000))
+      /* no sockets to wait on, get out of the loop */
+      break;
 
     timediff = Curl_tvdiff(Curl_tvnow(), now); /* spent time */
     timeout -= timediff?timediff:1; /* always deduct at least 1 */
diff --git a/lib/select.c b/lib/select.c
index daa591eae..28bb141b1 100644
--- a/lib/select.c
+++ b/lib/select.c
@@ -482,227 +482,6 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms)
   return r;
 }
 
-/*
- * This is a wrapper around select().  It uses poll() when a fine
- * poll() is available, in order to avoid limits with FD_SETSIZE,
- * otherwise select() is used.  An error is returned if select() is
- * being used and a the number of file descriptors is larger than
- * FD_SETSIZE.  A NULL timeout pointer makes this function wait
- * indefinitely, unles no valid file descriptor is given, when this
- * happens the NULL timeout is ignored and the function times out
- * immediately.  When compiled with CURL_ACKNOWLEDGE_EINTR defined,
- * EINTR condition is honored and function might exit early without
- * awaiting timeout, otherwise EINTR will be ignored.
- *
- * Return values:
- *   -1 = system call error or nfds > FD_SETSIZE
- *    0 = timeout
- *    N = number of file descriptors kept in file descriptor sets.
- */
-int Curl_select(int nfds,
-                fd_set *fds_read, fd_set *fds_write, fd_set *fds_excep,
-                struct timeval *timeout)
-{
-  struct timeval initial_tv;
-  int timeout_ms;
-  int pending_ms = 0;
-  int error;
-  int r;
-#ifdef HAVE_POLL_FINE
-  struct pollfd small_fds[SMALL_POLLNFDS];
-  struct pollfd *poll_fds;
-  int ix;
-  int fd;
-  int poll_nfds = 0;
-#else
-  struct timeval pending_tv;
-  struct timeval *ptimeout;
-#endif
-  int ret = 0;
-
-  if ((nfds < 0) ||
-     ((nfds > 0) && (!fds_read && !fds_write && !fds_excep))) {
-    SET_SOCKERRNO(EINVAL);
-    return -1;
-  }
-
-  if (timeout) {
-    if ((timeout->tv_sec < 0) ||
-        (timeout->tv_usec < 0) ||
-        (timeout->tv_usec >= 1000000)) {
-      SET_SOCKERRNO(EINVAL);
-      return -1;
-    }
-    timeout_ms = (int)(timeout->tv_sec * 1000) +
-      (int)(timeout->tv_usec / 1000);
-  }
-  else {
-    timeout_ms = -1;
-  }
-
-  if ((!nfds) || (!fds_read && !fds_write && !fds_excep)) {
-    r = wait_ms(timeout_ms);
-    return r;
-  }
-
-  /* Avoid initial timestamp, avoid gettimeofday() call, when elapsed
-     time in this function does not need to be measured. This happens
-     when function is called with a zero timeout in the timeval struct
-     referenced argument or when a NULL pointer is received as timeval
-     reference indicating a blocking call should be performed. */
-
-  if (timeout_ms > 0) {
-    pending_ms = timeout_ms;
-    initial_tv = curlx_tvnow();
-  }
-
-#ifdef HAVE_POLL_FINE
-
-  if (fds_read || fds_write || fds_excep) {
-    fd = nfds;
-    while (fd--) {
-      if ((fds_read && (0 != FD_ISSET(fd, fds_read))) ||
-          (fds_write && (0 != FD_ISSET(fd, fds_write))) ||
-          (fds_excep && (0 != FD_ISSET(fd, fds_excep))))
-        poll_nfds++;
-    }
-  }
-
-  if (!poll_nfds)
-    poll_fds = NULL;
-  else if (poll_nfds <= SMALL_POLLNFDS)
-    poll_fds = small_fds;
-  else {
-    poll_fds = malloc(poll_nfds * sizeof(struct pollfd));
-    if (!poll_fds) {
-      SET_SOCKERRNO(ENOBUFS);
-      return -1;
-    }
-  }
-
-  if (poll_fds) {
-    int events;
-    ix = 0;
-    fd = nfds;
-    while (fd--) {
-      events = 0;
-      if (fds_read && (0 != FD_ISSET(fd, fds_read)))
-        events |= (POLLRDNORM|POLLIN);
-      if (fds_write && (0 != FD_ISSET(fd, fds_write)))
-        events |= (POLLWRNORM|POLLOUT);
-      if (fds_excep && (0 != FD_ISSET(fd, fds_excep)))
-        events |= (POLLRDBAND|POLLPRI);
-      if (events) {
-        poll_fds[ix].events = events;
-        poll_fds[ix].fd = fd;
-        poll_fds[ix].revents = 0;
-        ix++;
-        if(ix == poll_nfds)
-          /* since we know this is the total amount of descriptors with
-             interesting actions, we can skip the rest of the loop at this
-             point */
-          break;
-      }
-    }
-  }
-
-  do {
-    if (timeout_ms < 0)
-      pending_ms = -1;
-    else if (!timeout_ms)
-      pending_ms = 0;
-    r = poll(poll_fds, poll_nfds, pending_ms);
-    if (r != -1)
-      break;
-    error = SOCKERRNO;
-    if ((error == EINVAL) || error_is_EINTR)
-      break;
-    if (timeout_ms > 0) {
-      pending_ms = timeout_ms - elapsed_ms;
-      if (pending_ms <= 0)
-        break;
-    }
-  } while (r == -1);
-
-  if (r < 0)
-    ret = -1;
-
-  if (r > 0) {
-    ix = poll_nfds;
-    while (ix--) {
-      if (poll_fds[ix].revents & POLLNVAL) {
-        SET_SOCKERRNO(EBADF);
-        ret = -1;
-        break;
-      }
-    }
-  }
-
-  if (!ret) {
-    ix = poll_nfds;
-    while (ix--) {
-      if (fds_read && (0 != FD_ISSET(poll_fds[ix].fd, fds_read))) {
-        if (0 == (poll_fds[ix].revents & (POLLRDNORM|POLLERR|POLLHUP|POLLIN)))
-          FD_CLR(poll_fds[ix].fd, fds_read);
-        else
-          ret++;
-      }
-      if (fds_write && (0 != FD_ISSET(poll_fds[ix].fd, fds_write))) {
-        if (0 == (poll_fds[ix].revents & (POLLWRNORM|POLLERR|POLLHUP|POLLOUT)))
-          FD_CLR(poll_fds[ix].fd, fds_write);
-        else
-          ret++;
-      }
-      if (fds_excep && (0 != FD_ISSET(poll_fds[ix].fd, fds_excep))) {
-        if (0 == (poll_fds[ix].revents & (POLLRDBAND|POLLERR|POLLHUP|POLLPRI)))
-          FD_CLR(poll_fds[ix].fd, fds_excep);
-        else
-          ret++;
-      }
-    }
-  }
-
-  if (poll_fds && (poll_nfds > SMALL_POLLNFDS))
-    free(poll_fds);
-
-#else  /* HAVE_POLL_FINE */
-
-  VERIFY_NFDS(nfds);
-
-  ptimeout = (timeout_ms < 0) ? NULL : &pending_tv;
-
-  do {
-    if (timeout_ms > 0) {
-      pending_tv.tv_sec = pending_ms / 1000;
-      pending_tv.tv_usec = (pending_ms % 1000) * 1000;
-    }
-    else if (!timeout_ms) {
-      pending_tv.tv_sec = 0;
-      pending_tv.tv_usec = 0;
-    }
-    r = select(nfds, fds_read, fds_write, fds_excep, ptimeout);
-    if (r != -1)
-      break;
-    error = SOCKERRNO;
-    if ((error == EINVAL) || (error == EBADF) || error_is_EINTR)
-      break;
-    if (timeout_ms > 0) {
-      pending_ms = timeout_ms - elapsed_ms;
-      if (pending_ms <= 0)
-        break;
-    }
-  } while (r == -1);
-
-  if (r < 0)
-    ret = -1;
-  else
-    ret = r;
-
-#endif  /* HAVE_POLL_FINE */
-
-  return ret;
-}
-
 #ifdef TPF
 /*
  * This is a replacement for select() on the TPF platform.
diff --git a/lib/select.h b/lib/select.h
index 77b3e915f..ce7b2f897 100644
--- a/lib/select.h
+++ b/lib/select.h
@@ -81,10 +81,6 @@ int Curl_socket_ready(curl_socket_t readfd, curl_socket_t writefd,
 
 int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms);
 
-int Curl_select(int nfds,
-                fd_set *fds_read, fd_set *fds_write, fd_set *fds_excep,
-                struct timeval *timeout);
-
 #ifdef TPF
 int tpf_select_libcurl(int maxfds, fd_set* reads, fd_set* writes,
                        fd_set* excepts, struct timeval* tv);