Moved all signal-based name resolution timeout handling into a single new

Curl_resolv_timeout function to reduce coupling.

3 files changed, 199 insertions, 138 deletions

diff --git a/lib/hostip.c b/lib/hostip.c
index 722c4b525..69f3c997f 100644
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -55,6 +55,9 @@
 #ifdef HAVE_SETJMP_H
 #include <setjmp.h>
 #endif
+#ifdef HAVE_SIGNAL_H
+#include <signal.h>
+#endif
 
 #ifdef HAVE_PROCESS_H
 #include <process.h>
@@ -76,6 +79,12 @@
 /* The last #include file should be: */
 #include "memdebug.h"
 
+#if defined(HAVE_ALARM) && defined(SIGALRM) && defined(HAVE_SIGSETJMP) \
+    && !defined(USE_ARES)
+/* alarm-based timeouts can only be used with all the dependencies satisfied */
+#define USE_ALARM_TIMEOUT
+#endif
+
 /*
  * hostip.c explained
  * ==================
@@ -388,19 +397,8 @@ int Curl_resolv(struct connectdata *conn,
   struct SessionHandle *data = conn->data;
   CURLcode result;
   int rc = CURLRESOLV_ERROR; /* default to failure */
-  *entry = NULL;
 
-#ifdef HAVE_SIGSETJMP
-  /* this allows us to time-out from the name resolver, as the timeout
-     will generate a signal and we will siglongjmp() from that here */
-  if(!data->set.no_signal) {
-    if(sigsetjmp(curl_jmpenv, 1)) {
-      /* this is coming from a siglongjmp() */
-      failf(data, "name lookup timed out");
-      return rc;
-    }
-  }
-#endif
+  *entry = NULL;
 
   /* Create an entry id, based upon the hostname and port */
   entry_id = create_hostcache_id(hostname, port);
@@ -484,6 +482,174 @@ int Curl_resolv(struct connectdata *conn,
   return rc;
 }
 
+#ifdef USE_ALARM_TIMEOUT 
+/*
+ * This signal handler jumps back into the main libcurl code and continues
+ * execution.  This effectively causes the remainder of the application to run
+ * within a signal handler which is nonportable and could lead to problems.
+ */
+static
+RETSIGTYPE alarmfunc(int sig)
+{
+  /* this is for "-ansi -Wall -pedantic" to stop complaining!   (rabe) */
+  (void)sig;
+  siglongjmp(curl_jmpenv, 1);
+  return;
+}
+#endif /* USE_ALARM_TIMEOUT */
+
+/*
+ * Curl_resolv_timeout() is the same as Curl_resolv() but specifies a
+ * timeout.  This function might return immediately if we're using asynch
+ * resolves. See the return codes.
+ *
+ * The cache entry we return will get its 'inuse' counter increased when this
+ * function is used. You MUST call Curl_resolv_unlock() later (when you're
+ * done using this struct) to decrease the counter again.
+ *
+ * If built with a synchronous resolver and use of signals is not
+ * disabled by the application, then a nonzero timeout will cause a
+ * timeout after the specified number of milliseconds. Otherwise, timeout
+ * is ignored.
+ *
+ * Return codes:
+ *
+ * CURLRESOLV_TIMEDOUT(-2) = warning, time too short or previous alarm expired
+ * CURLRESOLV_ERROR   (-1) = error, no pointer
+ * CURLRESOLV_RESOLVED (0) = OK, pointer provided
+ * CURLRESOLV_PENDING  (1) = waiting for response, no pointer
+ */
+
+int Curl_resolv_timeout(struct connectdata *conn,
+                        const char *hostname,
+                        int port,
+                        struct Curl_dns_entry **entry,
+                        long timeout)
+{
+#ifdef USE_ALARM_TIMEOUT 
+#ifdef HAVE_SIGACTION
+  struct sigaction keep_sigact;   /* store the old struct here */
+  bool keep_copysig=FALSE;        /* did copy it? */
+  struct sigaction sigact;
+#else
+#ifdef HAVE_SIGNAL
+  void (*keep_sigact)(int);       /* store the old handler here */
+#endif /* HAVE_SIGNAL */
+#endif /* HAVE_SIGACTION */
+
+  unsigned int prev_alarm=0;
+#endif /* USE_ALARM_TIMEOUT */
+
+  struct SessionHandle *data = conn->data;
+  int rc = CURLRESOLV_ERROR; /* error by default */
+
+  *entry = NULL;
+
+#ifdef USE_ALARM_TIMEOUT 
+  if (data->set.no_signal)
+    /* Ignore the timeout when signals are disabled */
+    timeout = 0;
+
+  if(timeout && timeout < 1000)
+    /* The alarm() function only provides integer second resolution, so if
+       we want to wait less than one second we must bail out already now. */
+    return CURLRESOLV_TIMEDOUT;
+
+  if (timeout > 0) {
+    /* This allows us to time-out from the name resolver, as the timeout
+       will generate a signal and we will siglongjmp() from that here.
+       This technique has problems (see alarmfunc). */
+      if(sigsetjmp(curl_jmpenv, 1)) {
+        /* this is coming from a siglongjmp() after an alarm signal */
+        failf(data, "name lookup timed out");
+        return rc;
+      }
+
+    /*************************************************************
+     * Set signal handler to catch SIGALRM
+     * Store the old value to be able to set it back later!
+     *************************************************************/
+#ifdef HAVE_SIGACTION
+    sigaction(SIGALRM, NULL, &sigact);
+    keep_sigact = sigact;
+    keep_copysig = TRUE; /* yes, we have a copy */
+    sigact.sa_handler = alarmfunc;
+#ifdef SA_RESTART
+    /* HPUX doesn't have SA_RESTART but defaults to that behaviour! */
+    sigact.sa_flags &= ~SA_RESTART;
+#endif
+    /* now set the new struct */
+    sigaction(SIGALRM, &sigact, NULL);
+#else /* HAVE_SIGACTION */
+    /* no sigaction(), revert to the much lamer signal() */
+#ifdef HAVE_SIGNAL
+    keep_sigact = signal(SIGALRM, alarmfunc);
+#endif
+#endif /* HAVE_SIGACTION */
+
+    /* alarm() makes a signal get sent when the timeout fires off, and that
+       will abort system calls */
+    prev_alarm = alarm((unsigned int) (timeout ? timeout/1000L : timeout));
+  }
+
+#else
+#ifndef CURLRES_ASYNCH
+  if(timeout)
+    infof(data, "timeout on name lookup is not supported\n");
+#endif
+#endif /* USE_ALARM_TIMEOUT */
+
+  /* Perform the actual name resolution. This might be interrupted by an
+   * alarm if it takes too long.
+   */
+  rc = Curl_resolv(conn, hostname, port, entry);
+
+#ifdef USE_ALARM_TIMEOUT 
+  if (timeout > 0) {
+
+#ifdef HAVE_SIGACTION
+    if(keep_copysig) {
+      /* we got a struct as it looked before, now put that one back nice
+         and clean */
+      sigaction(SIGALRM, &keep_sigact, NULL); /* put it back */
+    }
+#else
+#ifdef HAVE_SIGNAL
+    /* restore the previous SIGALRM handler */
+    signal(SIGALRM, keep_sigact);
+#endif
+#endif /* HAVE_SIGACTION */
+
+    /* switch back the alarm() to either zero or to what it was before minus
+       the time we spent until now! */
+    if(prev_alarm) {
+      /* there was an alarm() set before us, now put it back */
+      unsigned long elapsed_ms = Curl_tvdiff(Curl_tvnow(), conn->created);
+
+      /* the alarm period is counted in even number of seconds */
+      unsigned long alarm_set = prev_alarm - elapsed_ms/1000;
+
+      if(!alarm_set ||
+         ((alarm_set >= 0x80000000) && (prev_alarm < 0x80000000)) ) {
+        /* if the alarm time-left reached zero or turned "negative" (counted
+           with unsigned values), we should fire off a SIGALRM here, but we
+           won't, and zero would be to switch it off so we never set it to
+           less than 1! */
+        alarm(1);
+        rc = CURLRESOLV_TIMEDOUT;
+        failf(data, "Previous alarm fired off!");
+      }
+      else
+        alarm((unsigned int)alarm_set);
+    }
+    else
+      alarm(0); /* just shut it off */
+  }
+#endif /* USE_ALARM_TIMEOUT */
+
+  return rc;
+}
+
 /*
  * Curl_resolv_unlock() unlocks the given cached DNS entry. When this has been
  * made, the struct may be destroyed due to pruning. It is important that only
diff --git a/lib/hostip.h b/lib/hostip.h
index c6172be0f..3864dd90b 100644
--- a/lib/hostip.h
+++ b/lib/hostip.h
@@ -158,11 +158,15 @@ struct Curl_dns_entry {
  * use, or we'll leak memory!
  */
 /* return codes */
+#define CURLRESOLV_TIMEDOUT -2
 #define CURLRESOLV_ERROR    -1
 #define CURLRESOLV_RESOLVED  0
 #define CURLRESOLV_PENDING   1
 int Curl_resolv(struct connectdata *conn, const char *hostname,
                 int port, struct Curl_dns_entry **dnsentry);
+int Curl_resolv_timeout(struct connectdata *conn, const char *hostname,
+                        int port, struct Curl_dns_entry **dnsentry,
+                        long timeout);
 
 /*
  * Curl_ipvalid() checks what CURL_IPRESOLVE_* requirements that might've
diff --git a/lib/url.c b/lib/url.c
index 53e2bf7e0..df3e3cafc 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -60,9 +60,6 @@
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
 #endif
-#ifdef HAVE_SIGNAL_H
-#include <signal.h>
-#endif
 
 #ifdef HAVE_SYS_PARAM_H
 #include <sys/param.h>
@@ -73,10 +70,6 @@
 #include <inet.h>
 #endif
 
-#ifdef HAVE_SETJMP_H
-#include <setjmp.h>
-#endif
-
 #ifndef HAVE_SOCKET
 #error "We can't compile without socket() support!"
 #endif
@@ -169,27 +162,6 @@ static void flush_cookies(struct SessionHandle *data, int cleanup);
 #define verboseconnect(x)  do { } while (0)
 #endif
 
-#ifndef WIN32
-/* not for WIN32 builds */
-
-#if defined(HAVE_ALARM) && defined(SIGALRM) && !defined(USE_ARES)
-/*
- * This signal handler jumps back into the main libcurl code and continues
- * execution.  This effectively causes the remainder of the application to run
- * within a signal handler which is nonportable and could lead to problems.
- */
-static
-RETSIGTYPE alarmfunc(int sig)
-{
-  /* this is for "-ansi -Wall -pedantic" to stop complaining!   (rabe) */
-  (void)sig;
-#ifdef HAVE_SIGSETJMP
-  siglongjmp(curl_jmpenv, 1);
-#endif
-  return;
-}
-#endif /* HAVE_ALARM && SIGALRM && !USE_ARES */
-#endif /* WIN32 */
 
 /*
  * Protocol table.
@@ -3788,34 +3760,19 @@ static CURLcode resolve_server(struct SessionHandle *data,
                                bool *async)
 {
   CURLcode result=CURLE_OK;
-
-#if defined(HAVE_ALARM) && defined(SIGALRM) && !defined(USE_ARES)
-#ifdef HAVE_SIGACTION
-  struct sigaction keep_sigact;   /* store the old struct here */
-  bool keep_copysig=FALSE;        /* did copy it? */
-#else
-#ifdef HAVE_SIGNAL
-  void (*keep_sigact)(int);       /* store the old handler here */
-#endif /* HAVE_SIGNAL */
-#endif /* HAVE_SIGACTION */
-
-  unsigned int prev_alarm=0;
+  long shortest = 0; /* default to no timeout */
 
   /*************************************************************
-   * Set timeout if that is being used, and we're not using an asynchronous
-   * name resolve.
+   * Set timeout if that is being used
    *************************************************************/
-  if((data->set.timeout || data->set.connecttimeout) && !data->set.no_signal) {
-#ifdef HAVE_SIGACTION
-    struct sigaction sigact;
-#endif /* HAVE_SIGACTION */
+  if(data->set.timeout || data->set.connecttimeout) {
 
     /* We set the timeout on the name resolving phase first, separately from
      * the download/upload part to allow a maximum time on everything. This is
      * a signal-based timeout, why it won't work and shouldn't be used in
      * multi-threaded environments. */
 
-    long shortest = data->set.timeout; /* default to this timeout value */
+    shortest = data->set.timeout; /* default to this timeout value */
     if(shortest && data->set.connecttimeout &&
        (data->set.connecttimeout < shortest))
       /* if both are set, pick the shortest */
@@ -3823,42 +3780,10 @@ static CURLcode resolve_server(struct SessionHandle *data,
     else if(!shortest)
       /* if timeout is not set, use the connect timeout */
       shortest = data->set.connecttimeout;
-
-    if(shortest < 1000)
-      /* the alarm() function only provide integer second resolution, so if
-         we want to wait less than one second we must bail out already now. */
-      return CURLE_OPERATION_TIMEDOUT;
-
-    /*************************************************************
-     * Set signal handler to catch SIGALRM
-     * Store the old value to be able to set it back later!
-     *************************************************************/
-#ifdef HAVE_SIGACTION
-    sigaction(SIGALRM, NULL, &sigact);
-    keep_sigact = sigact;
-    keep_copysig = TRUE; /* yes, we have a copy */
-    sigact.sa_handler = alarmfunc;
-#ifdef SA_RESTART
-    /* HPUX doesn't have SA_RESTART but defaults to that behaviour! */
-    sigact.sa_flags &= ~SA_RESTART;
-#endif
-    /* now set the new struct */
-    sigaction(SIGALRM, &sigact, NULL);
-#else /* HAVE_SIGACTION */
-    /* no sigaction(), revert to the much lamer signal() */
-#ifdef HAVE_SIGNAL
-    keep_sigact = signal(SIGALRM, alarmfunc);
-#endif
-#endif /* HAVE_SIGACTION */
-
-    /* alarm() makes a signal get sent when the timeout fires off, and that
-       will abort system calls */
-    prev_alarm = alarm((unsigned int) (shortest ? shortest/1000L : shortest));
-    /* We can expect the conn->created time to be "now", as that was just
-       recently set in the beginning of this function and nothing slow
-       has been done since then until now. */
+  /* We can expect the conn->created time to be "now", as that was just
+     recently set in the beginning of this function and nothing slow
+     has been done since then until now. */
   }
-#endif /* HAVE_ALARM && SIGALRM && !USE_ARES */
 
   /*************************************************************
    * Resolve the name of the server or proxy
@@ -3885,10 +3810,14 @@ static CURLcode resolve_server(struct SessionHandle *data,
       conn->port =  conn->remote_port; /* it is the same port */
 
       /* Resolve target host right on */
-      rc = Curl_resolv(conn, conn->host.name, (int)conn->port, &hostaddr);
+      rc = Curl_resolv_timeout(conn, conn->host.name, (int)conn->port,
+                               &hostaddr, shortest);
       if(rc == CURLRESOLV_PENDING)
         *async = TRUE;
 
+      else if (rc == CURLRESOLV_TIMEDOUT)
+        result = CURLE_OPERATION_TIMEDOUT;
+
       else if(!hostaddr) {
         failf(data, "Couldn't resolve host '%s'", conn->host.dispname);
         result =  CURLE_COULDNT_RESOLVE_HOST;
@@ -3902,11 +3831,15 @@ static CURLcode resolve_server(struct SessionHandle *data,
       fix_hostname(data, conn, &conn->proxy);
 
       /* resolve proxy */
-      rc = Curl_resolv(conn, conn->proxy.name, (int)conn->port, &hostaddr);
+      rc = Curl_resolv_timeout(conn, conn->proxy.name, (int)conn->port,
+                               &hostaddr, shortest);
 
       if(rc == CURLRESOLV_PENDING)
         *async = TRUE;
 
+      else if (rc == CURLRESOLV_TIMEDOUT)
+        result = CURLE_OPERATION_TIMEDOUT;
+
       else if(!hostaddr) {
         failf(data, "Couldn't resolve proxy '%s'", conn->proxy.dispname);
         result = CURLE_COULDNT_RESOLVE_PROXY;
@@ -3916,48 +3849,6 @@ static CURLcode resolve_server(struct SessionHandle *data,
     *addr = hostaddr;
   }
 
-#if defined(HAVE_ALARM) && defined(SIGALRM) && !defined(USE_ARES)
-  if((data->set.timeout || data->set.connecttimeout) && !data->set.no_signal) {
-#ifdef HAVE_SIGACTION
-    if(keep_copysig) {
-      /* we got a struct as it looked before, now put that one back nice
-         and clean */
-      sigaction(SIGALRM, &keep_sigact, NULL); /* put it back */
-    }
-#else
-#ifdef HAVE_SIGNAL
-    /* restore the previous SIGALRM handler */
-    signal(SIGALRM, keep_sigact);
-#endif
-#endif /* HAVE_SIGACTION */
-
-    /* switch back the alarm() to either zero or to what it was before minus
-       the time we spent until now! */
-    if(prev_alarm) {
-      /* there was an alarm() set before us, now put it back */
-      unsigned long elapsed_ms = Curl_tvdiff(Curl_tvnow(), conn->created);
-      unsigned long alarm_set;
-
-      /* the alarm period is counted in even number of seconds */
-      alarm_set = prev_alarm - elapsed_ms/1000;
-
-      if(!alarm_set ||
-         ((alarm_set >= 0x80000000) && (prev_alarm < 0x80000000)) ) {
-        /* if the alarm time-left reached zero or turned "negative" (counted
-           with unsigned values), we should fire off a SIGALRM here, but we
-           won't, and zero would be to switch it off so we never set it to
-           less than 1! */
-        alarm(1);
-        result = CURLE_OPERATION_TIMEDOUT;
-        failf(data, "Previous alarm fired off!");
-      }
-      else
-        alarm((unsigned int)alarm_set);
-    }
-    else
-      alarm(0); /* just shut it off */
-  }
-#endif /* HAVE_ALARM && SIGALRM && !USE_ARES */
   return result;
 }