myalarm() is history, we now use HAVE_ALARM and we now do our very best to

1 - restore the previous sigaction struct as soon as we are about to shut
off our timeout
2 - restore the previous alarm() timeout, in case an application or similar
had it running before we "borrowed" it for a while.

No, this does not fix the multi-thread problem you get with alarm(). This
patch should correct bug report #478780:
//sourceforge.net/tracker/?func=detail&atid=100976&aid=478780&group_id=976

If not, please post details!

3 files changed, 82 insertions, 40 deletions

diff --git a/lib/setup.h b/lib/setup.h
index 6576919ea..30e945c2f 100644
--- a/lib/setup.h
+++ b/lib/setup.h
@@ -110,13 +110,13 @@ defined(HAVE_LIBSSL) && defined(HAVE_LIBCRYPTO)
 #define sclose(x) closesocket(x)
 #define sread(x,y,z) recv(x,y,z,0)
 #define swrite(x,y,z) (size_t)send(x,y,z,0)
-#define myalarm(x) /* win32 is a silly system */
+#undef HAVE_ALARM
 #else
      /* gcc-for-win is still good :) */
 #define sclose(x) close(x)
 #define sread(x,y,z) recv(x,y,z,0)
 #define swrite(x,y,z) send(x,y,z,0)
-#define myalarm(x) alarm(x)
+#define HAVE_ALARM
 #endif
 
 #define PATH_CHAR     ";"
@@ -127,7 +127,7 @@ defined(HAVE_LIBSSL) && defined(HAVE_LIBCRYPTO)
 #define sclose(x) close(x)
 #define sread(x,y,z) recv(x,y,z,0)
 #define swrite(x,y,z) send(x,y,z,0)
-#define myalarm(x) alarm(x)
+#define HAVE_ALARM
 
 #define PATH_CHAR     ":"
 #define DIR_CHAR      "/"
diff --git a/lib/transfer.c b/lib/transfer.c
index 449b2e6eb..2d1c90f6a 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1159,10 +1159,6 @@ CURLcode Curl_perform(struct SessionHandle *data)
   if(newurl)
     free(newurl);
 
-  /* make absolutely sure the alarm is switched off! */
-  if(data->set.timeout || data->set.connecttimeout)
-    myalarm(0);
-
   return res;
 }
 
diff --git a/lib/url.c b/lib/url.c
index 6b2a3a8c2..5810e859b 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -72,7 +72,6 @@
 #include <inet.h>
 #endif
 
-
 #ifndef HAVE_SELECT
 #error "We can't compile without select() support!"
 #endif
@@ -200,10 +199,6 @@ CURLcode Curl_open(struct SessionHandle **curl)
 {
   /* We don't yet support specifying the URL at this point */
   struct SessionHandle *data;
-#ifdef HAVE_SIGACTION
-  struct sigaction sigact;
-#endif
-
   /* Very simple start-up: alloc the struct, init it with zeroes and return */
   data = (struct SessionHandle *)malloc(sizeof(struct SessionHandle));
   if(!data)
@@ -264,24 +259,6 @@ CURLcode Curl_open(struct SessionHandle **curl)
   *curl = data;
 
   /*************************************************************
-   * Set signal handler to catch SIGALRM
-   *************************************************************/
-#ifdef HAVE_SIGACTION
-  sigaction(SIGALRM, NULL, &sigact);
-  sigact.sa_handler = alarmfunc;
-#ifdef SA_RESTART
-  /* HPUX doesn't have SA_RESTART but defaults to that behaviour! */
-  sigact.sa_flags &= ~SA_RESTART;
-#endif
-  sigaction(SIGALRM, &sigact, NULL);
-#else
-  /* no sigaction(), revert to the much lamer signal() */
-#ifdef HAVE_SIGNAL
-  signal(SIGALRM, alarmfunc);
-#endif
-#endif
-
-  /*************************************************************
    * Tell signal handler to ignore SIGPIPE
    *************************************************************/
 #if defined(HAVE_SIGNAL) && defined(SIGPIPE)
@@ -1170,6 +1147,14 @@ static CURLcode CreateConnection(struct SessionHandle *data,
   struct connectdata *conn_temp;
   char endbracket;
   int urllen;
+#ifdef HAVE_ALARM
+  unsigned int prev_alarm;
+#endif
+
+#ifdef HAVE_SIGACTION
+  struct sigaction keep_sigact;   /* store the old struct here */
+  bool keep_copysig;              /* did copy it? */
+#endif
 
   /*************************************************************
    * Check input data
@@ -1937,17 +1922,43 @@ static CURLcode CreateConnection(struct SessionHandle *data,
    * Set timeout if that is being used
    *************************************************************/
   if(data->set.timeout || data->set.connecttimeout) {
+    /*************************************************************
+     * Set signal handler to catch SIGALRM
+     * Store the old value to be able to set it back later!
+     *************************************************************/
+
+#ifdef HAVE_SIGACTION
+    struct sigaction sigact;
+    sigaction(SIGALRM, NULL, &sigact);
+    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
+    /* no sigaction(), revert to the much lamer signal() */
+#ifdef HAVE_SIGNAL
+    signal(SIGALRM, alarmfunc);
+#endif
+#endif
+
     /* 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. */
 
-    /* myalarm() makes a signal get sent when the timeout fires off, and that
+#ifdef HAVE_ALARM
+    /* alarm() makes a signal get sent when the timeout fires off, and that
        will abort system calls */
-    if(data->set.connecttimeout)
-      myalarm(data->set.connecttimeout);
-    else
-      myalarm(data->set.timeout);
+    prev_alarm = alarm(data->set.connecttimeout?
+                       data->set.connecttimeout:
+                       data->set.timeout);
+    /* 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
   }
 
   /*************************************************************
@@ -1966,7 +1977,8 @@ static CURLcode CreateConnection(struct SessionHandle *data,
     }
     if(!conn->hostaddr) {
       failf(data, "Couldn't resolve host '%s'", conn->name);
-      return CURLE_COULDNT_RESOLVE_HOST;
+      result =  CURLE_COULDNT_RESOLVE_HOST;
+      /* don't return yet, we need to clean up the timeout first */
     }
   }
   else if(!conn->hostaddr) {
@@ -1980,13 +1992,47 @@ static CURLcode CreateConnection(struct SessionHandle *data,
 
     if(!conn->hostaddr) {
       failf(data, "Couldn't resolve proxy '%s'", conn->proxyhost);
-      return CURLE_COULDNT_RESOLVE_PROXY;
+      result = CURLE_COULDNT_RESOLVE_PROXY;
+      /* don't return yet, we need to clean up the timeout first */
     }
   }
   Curl_pgrsTime(data, TIMER_NAMELOOKUP);
-  if(data->set.timeout || data->set.connecttimeout)
-    /* switch off signal-based timeouts */
-    myalarm(0);
+#ifdef HAVE_ALARM
+  if(data->set.timeout || data->set.connecttimeout) {
+#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 */
+    }
+#endif
+    /* 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 */
+      long elapsed_ms = Curl_tvdiff(Curl_tvnow(), conn->created);
+      long alarm_set;
+
+      /* the alarm period is counted in even number of seconds */
+      alarm_set = prev_alarm - elapsed_ms/1000;
+
+      if(alarm_set<=0) {
+        /* if it turned negative, 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_TIMEOUTED;
+        failf(data, "Previous alarm fired off!");
+      }
+      else
+        alarm(alarm_set);
+    }
+    else
+      alarm(0); /* just shut it off */
+  }
+#endif
+  if(result)
+    return result;
 
   /*************************************************************
    * Proxy authentication