- Bryan Henderson introduces two things:

  1) the progress callback gets called more frequently (at times)
  2) libcurl *might* call the callback when it receives a signal

4 files changed, 197 insertions, 41 deletions

diff --git a/lib/progress.c b/lib/progress.c
index 5ba829a03..65bd4a7bb 100644
--- a/lib/progress.c
+++ b/lib/progress.c
@@ -280,9 +280,6 @@ int Curl_pgrsUpdate(struct connectdata *conn)
     ((double)data->progress.uploaded/
      (data->progress.timespent>0?data->progress.timespent:1));
 
-  if(data->progress.lastshow == Curl_tvlong(now))
-    return 0; /* never update this more than once a second if the end isn't
-                 reached */
   data->progress.lastshow = now.tv_sec;
 
   /* Let's do the "current speed" thing, which should use the fastest
@@ -359,6 +356,10 @@ int Curl_pgrsUpdate(struct connectdata *conn)
     return result;
   }
 
+  if(data->progress.lastshow == Curl_tvlong(now))
+    return 0; /* never update this more than once a second if the end isn't
+                 reached */
+
   /* Figure out the estimated time of arrival for the upload */
   if((data->progress.flags & PGRS_UL_SIZE_KNOWN) &&
      (data->progress.ulspeed>0) &&
diff --git a/lib/select.c b/lib/select.c
index 4f8878401..a1e149a12 100644
--- a/lib/select.c
+++ b/lib/select.c
@@ -32,9 +32,7 @@
 #include <sys/time.h>
 #endif
 
-#ifndef HAVE_SELECT
-#error "We can't compile without select() support!"
-#endif
+#include <signal.h>
 
 #ifdef __BEOS__
 /* BeOS has FD_SET defined in socket.h */
@@ -53,6 +51,57 @@
 
 /* Winsock and TPF sockets are not in range [0..FD_SETSIZE-1] */
 
+/*  There are various ways to wait for a socket to be ready to give or take
+ *  data.  None of them are perfect.  
+ *
+ *  select() is available everywhere, but cannot take a file
+ *  descriptor numerically greater than FD_SETSIZE but cannot be reliably
+ *  interrupted by a signal.
+ *
+ *  pselect() works with signals, but still has the file descriptor problem.
+ *  And some older systems don't have it.
+ *
+ *  poll() (and equivalently on Windows, WSAPoll()) can take any file
+ *  descriptor, but has the signal problem.  And some older systems
+ *  don't have it.
+ *
+ *  The signal issue is this:  We would like to be able to avoid the
+ *  wait if a signal has arrived since we last checked for it.  All
+ *  these methods terminate the wait (with EINTR) if a signal arrives
+ *  while the waiting is underway, so it's just signals that happen
+ *  shortly before the wait that are a problem.  With pselect(), this
+ *  is possible because it has the ability to simultaneously unblock
+ *  signals _after_ the wait begins.  So you just block signals, then
+ *  check for arrival, then assuming no signals have arrived, call
+ *  pselect() with an argument that says to unblock signals.  Any
+ *  signal that arrived after you blocked will thus interrupt the wait
+ *  and pselect() returns immediately.
+ * 
+ *  Curl_pselect() is our compromise among these.  We use poll()
+ *  whenever it is available and select() otherwise.  We emulate
+ *  pselect-like signal behavior by unblocking signals just before
+ *  calling poll() or select() and re-blocking after.  This only
+ *  _approximates_ pselect(), because there is a window in which a
+ *  signal may arrive and we wait anyway.
+ *
+ *  To reduce that window, we use pselect(), if it is available --
+ *  with no file descriptors -- just before the poll() or select() in
+ *  order to detect signals that arrived between when the caller
+ *  blocked signals and when he called Curl_pselect().
+ *
+ *  Curl_select() is for callers who want us to ignore caught signals and
+ *  wait until a socket is ready or the timeout expires.  We implement that
+ *  simply as a loop around Curl_pselect().
+ *
+ *  There is a way to add signal interruptibility to poll(), which we
+ *  don't provide today: Let caller give us a file descriptor to add
+ *  to our list of wait-for-readable file descriptors.  Caller passes
+ *  us the fd of a pipe.  He doesn't block signals and his signal
+ *  handler writes to the other end of that pipe.  Therefore, a signal
+ *  causes poll() to return, even if received before poll() was
+ *  called.
+ */
+
 #if defined(USE_WINSOCK) || defined(TPF)
 #define VERIFY_SOCK(x) do { } while (0)
 #else
@@ -66,18 +115,56 @@
 #endif
 
 /*
- * This is an internal function used for waiting for read or write
- * events on single file descriptors.  It attempts to replace select()
- * in order to avoid limits with FD_SETSIZE.
+ * This function unblocks a set of signal classes momentarily, to allow any
+ * the process to receive any presently blocked signal.  If there exists
+ * a handler for that, it will run now.  If not, it will typically
+ * terminate the process.
  *
- * Return values:
- *   -1 = system call error
- *    0 = timeout
- *    CSELECT_IN | CSELECT_OUT | CSELECT_ERR
+ * We return 1 if as a result of the unblocking, a signal was
+ * received, caught and handled.  0 otherwise.
+ *
+ * On a system that does not have pselect(), we always return 0, even if
+ * signals were received.
  */
-int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms)
+int receive_signals(sigset_t * sigmask)
 {
+#ifdef HAVE_PSELECT
+  struct timespec zeroTime = {0, 0};
+
+  /* Note that on older Linux, pselect() is imperfect -- the kernel doesn't
+     have a pselect() system call, so the GNU C Library implements it
+     with sigprocmask() followed by select(), which means the result is
+     the same as with the code below for systmes with no pselect() at all.
+  */
+  if (pselect(0, NULL, NULL, NULL, &zeroTime, sigmask) == 0)
+      return 0;
+  else
+      return 1;
+#else
+  sigset_t oldmask;
+
+  sigprocmask(SIG_SETMASK, sigmask, &oldmask);
+  sigprocmask(SIG_SETMASK, &oldmask, NULL);
+
+  return 0;
+#endif
+}
+
 #if defined(HAVE_POLL_FINE) || defined(CURL_HAVE_WSAPOLL)
+  #define USE_POLL_FOR_SELECT 1
+#else
+  #if defined(HAVE_SELECT)
+    #define USE_POLL_FOR_SELECT 0
+  #else
+    #error "You don't appear to have either poll() or select()."
+  #endif
+#endif
+
+#if USE_POLL_FOR_SELECT
+
+static int select_with_poll(curl_socket_t readfd, curl_socket_t writefd,
+                            int timeout_ms)
+{
   struct pollfd pfd[2];
   int num;
   int r;
@@ -95,13 +182,11 @@ int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms)
     num++;
   }
 
-  do {
 #ifdef CURL_HAVE_WSAPOLL
-    r = WSAPoll(pfd, num, timeout_ms);
+  r = WSAPoll(pfd, num, timeout_ms);
 #else
-    r = poll(pfd, num, timeout_ms);
+  r = poll(pfd, num, timeout_ms);
 #endif
-  } while((r == -1) && (SOCKERRNO == EINTR));
 
   if (r < 0)
     return -1;
@@ -132,7 +217,13 @@ int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms)
   }
 
   return ret;
-#else
+}
+
+#endif USE_POLL_FOR_SELECT
+
+static int select_with_select(curl_socket_t readfd, curl_socket_t writefd,
+                              int timeout_ms)
+{
   struct timeval timeout;
   fd_set fds_read;
   fd_set fds_write;
@@ -179,9 +270,7 @@ int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms)
       maxfd = writefd;
   }
 
-  do {
-    r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, &timeout);
-  } while((r == -1) && (SOCKERRNO == EINTR));
+  r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, &timeout);
 
   if (r < 0)
     return -1;
@@ -203,7 +292,49 @@ int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms)
   }
 
   return ret;
+}
+
+/*
+ * This is an internal function used for waiting for read or write
+ * events on single file descriptors.  It attempts to replace select()
+ * in order to avoid limits with FD_SETSIZE.
+ *
+ * Return values:
+ *   -1 = system call error, including interrupted by signal
+ *    0 = timeout
+ *    CSELECT_IN | CSELECT_OUT | CSELECT_ERR
+ */
+int Curl_pselect(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms,
+                 sigset_t * sigmask)
+{
+  int ret;
+  sigset_t oldmask;
+
+  if (sigmask && receive_signals(sigmask)) {
+      SET_SOCKERRNO(EINTR);
+      ret = -1;
+  } else {
+    if (sigmask)
+      sigprocmask(SIG_SETMASK, sigmask, &oldmask);
+#if USE_POLL_FOR_SELECT
+    ret = select_with_poll(readfd, writefd, timeout_ms);
+#else
+    ret = select_with_select(readfd, writefd, timeout_ms);
 #endif
+    if (sigmask)
+      sigprocmask(SIG_SETMASK, &oldmask, NULL);
+  }
+  return ret;
+}
+
+int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms)
+{
+  int r;
+  do {
+    r = Curl_pselect(readfd, writefd, timeout_ms, NULL);
+  } while((r == -1) && (SOCKERRNO == EINTR));
+
+  return r;
 }
 
 /*
diff --git a/lib/select.h b/lib/select.h
index e0844b1d6..0941db6df 100644
--- a/lib/select.h
+++ b/lib/select.h
@@ -51,6 +51,9 @@ struct pollfd
 #define CSELECT_OUT  0x02
 #define CSELECT_ERR  0x04
 
+int Curl_pselect(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms,
+                 sigset_t * sigmask);
+
 int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms);
 
 int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms);
diff --git a/lib/transfer.c b/lib/transfer.c
index 8e6f5e19f..07e135ff1 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1603,8 +1603,6 @@ CURLcode Curl_readwrite(struct connectdata *conn,
       failf(data, "transfer closed with outstanding read data remaining");
       return CURLE_PARTIAL_FILE;
     }
-    if(Curl_pgrsUpdate(conn))
-      return CURLE_ABORTED_BY_CALLBACK;
   }
 
   /* Now update the "done" boolean we return */
@@ -1754,6 +1752,18 @@ int Curl_single_getsock(struct connectdata *conn,
 }
 
 
+
+static bool
+errnoIsInterruption(int errnoarg)
+{
+#ifdef EINTR
+  return (errnoarg == EINTR);
+#else
+  return FALSE;
+#endif
+}
+
+
 /*
  * Transfer()
  *
@@ -1775,6 +1785,12 @@ Transfer(struct connectdata *conn)
   struct SessionHandle *data = conn->data;
   struct Curl_transfer_keeper *k = &data->reqdata.keep;
   bool done=FALSE;
+  sigset_t callersigmask;
+  sigset_t allsignals;
+  int pgrsrc;
+  int selectrc;
+
+  sigfillset(&allsignals);
 
   if(!(conn->protocol & PROT_FILE)) {
     /* Only do this if we are not transferring FILE:, since the file: treatment
@@ -1828,28 +1844,33 @@ Transfer(struct connectdata *conn)
        the timeout case and if we limit transfer speed we must make sure that
        this function doesn't transfer anything while in HOLD status. */
 
-    switch (Curl_select(fd_read, fd_write, 1000)) {
-    case -1: /* select() error, stop reading */
-#ifdef EINTR
-      /* The EINTR is not serious, and it seems you might get this more
-         ofen when using the lib in a multi-threaded environment! */
-      if(SOCKERRNO == EINTR)
-        ;
-      else
-#endif
-        done = TRUE; /* no more read or write */
+    sigprocmask(SIG_SETMASK, &allsignals, &callersigmask);
+
+    pgrsrc = Curl_pgrsUpdate(conn);
+
+    if(!pgrsrc)
+      selectrc = Curl_pselect(fd_read, fd_write, 3000, &callersigmask);
+
+    sigprocmask(SIG_SETMASK, &callersigmask, NULL);
+
+    if(pgrsrc)
+      return CURLE_ABORTED_BY_CALLBACK;
+
+    if (selectrc == -1 && !errnoIsInterruption(SOCKERRNO)) {
+      done = TRUE; /* no more read or write */
       continue;
-    case 0:  /* timeout */
-    default: /* readable descriptors */
+    } else {
+      /* ready files, timeout, or signal received */
 
       result = Curl_readwrite(conn, &done);
-      break;
-    }
-    if(result)
-      return result;
 
-    /* "done" signals to us if the transfer(s) are ready */
+      /* "done" signals to us if the transfer(s) are ready */
+
+      if(result)
+        return result;
+    }
   }
+  Curl_pgrsUpdate(conn);
 
   return CURLE_OK;
 }