reverted the pselect patch => http://curl.haxx.se/mail/lib-2007-03/0100.html

8 files changed, 49 insertions, 253 deletions

diff --git a/CHANGES b/CHANGES
index 7c473de95..3433be3c5 100644
--- a/CHANGES
+++ b/CHANGES
@@ -27,28 +27,6 @@ Daniel (10 March 2007)
   problems both for HTTP pulling and cloning. Repository size is about 250 Mb,
   so it was a considerable amount of Curl's work.
 
-- 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:
-
-  libcurl calls the progress callback at least once a second, and sometimes
-  when the process receives and catches a signal.  Ideally, it would get
-  called every time the process receives and catches a signal, but in the
-  current implementation, libcurl may fail to recognize a signal during name
-  resolution, during the wait for a TCP connection, and during some tiny
-  windows other times.
-
-  If you want a signal to interrupt your call to libcurl, install a signal
-  handler for it.  Have that signal handler set a flag indicating that the
-  signal was received.  Set up a libcurl progress callback that checks that
-  flag and, if it is set, returns a nonzero return code.
-
-  Two common kinds of signals you might want to allow to interrupt libcurl
-  are: 1) SIGINT, the signal that typically results from a user typing
-  control-C; 2) SIGALRM, a signal indicating a timeout.  (libcurl also has
-  specific timeout facilities, but SIGALRM can be from a master timeout
-  established at a higher layer of your program).
-
 Dan F (9 March 2007)
 - Updated the test harness to add a new "crypto" feature check and updated the
   appropriate test case to use it.  For now, this is treated the same as the
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 2d5030026..70296814e 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -39,8 +39,6 @@ This release includes the following bugfixes:
  o HTTP Digest header parsing fix for unquoted last word ending with CRLF
  o CURLOPT_PORT, HTTP proxy, re-using connections and non-HTTP protocols
  o CURLOPT_INTERFACE for ipv6
- o the progress callback can get called more frequently
- o libcurl might call the progress callback when it receives a signal
  o use-after-free issue with HTTP transfers with the multi interface
 
 This release includes the following known bugs:
diff --git a/configure.ac b/configure.ac
index 8e6cb9940..22c86e36e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1794,7 +1794,6 @@ esac
 AC_CHECK_FUNCS( strtoll \
                 socket \
                 select \
-                pselect \
                 strdup \
                 strstr \
                 strtok_r \
diff --git a/docs/libcurl/curl_easy_setopt.3 b/docs/libcurl/curl_easy_setopt.3
index e949f2109..a5aa25b8c 100644
--- a/docs/libcurl/curl_easy_setopt.3
+++ b/docs/libcurl/curl_easy_setopt.3
@@ -67,9 +67,8 @@ A non-zero parameter tells the library to include the header in the body
 output. This is only relevant for protocols that actually have headers
 preceding the data (like HTTP).
 .IP CURLOPT_NOPROGRESS
-A non-zero parameter tells the library not to call your progress callback
-(see \fICURLOPT_PROGRESSFUNCTION\fP)
-and to shut off the built-in progress meter completely.
+A non-zero parameter tells the library to shut off the built-in progress meter
+completely.
 
 Future versions of libcurl is likely to not have any built-in progress meter
 at all.
@@ -186,45 +185,23 @@ argument in the sockopt callback set with \fICURLOPT_SOCKOPTFUNCTION\fP.
 .IP CURLOPT_PROGRESSFUNCTION
 Function pointer that should match the \fIcurl_progress_callback\fP prototype
 found in \fI<curl/curl.h>\fP. This function gets called by libcurl instead of
-its internal equivalent frequently during operation (roughly
+its internal equivalent with a frequent interval during operation (roughly
 once per second) no matter if data is being transfered or not.  Unknown/unused
 argument values passed to the callback will be set to zero (like if you only
-download data, the upload size will remain 0).
-
-The callback serves two purposes: 1) updates you on the progress of
-the transfer; 2) gives you an opportunity to abort the transfer.  If
-the callback returns a non-zero value, libcurl aborts the transfer and
-returns \fICURLE_ABORTED_BY_CALLBACK\fP.
-
-libcurl calls the progress callback at least once a second, and
-sometimes when the process receives and catches a signal.  Ideally, it 
-would get called every time the process receives and catches a signal,
-but in the current implementation, libcurl may fail to recognize a signal
-during name resolution, during the wait for a TCP connection, and during
-some tiny windows other times.
-
-If you want a signal to interrupt your call to libcurl, install a signal
-handler for it.  Have that signal handler set a flag indicating that the
-signal was received.  Set up a libcurl progress callback that checks that
-flag and, if it is set, returns a nonzero return code.
-
-Two common kinds of signals you might want to allow to interrupt
-libcurl are: 1) SIGINT, the signal that typically results from a user
-typing control-C; 2) SIGALRM, a signal indicating a timeout.  (libcurl
-also has specific timeout facilities, but SIGALRM can be from a master
-timeout established at a higher layer of your program).
+download data, the upload size will remain 0). Returning a non-zero value from
+this callback will cause libcurl to abort the transfer and return
+\fICURLE_ABORTED_BY_CALLBACK\fP.
 
 If you transfer data with the multi interface, this function will not be
 called during periods of idleness unless you call the appropriate libcurl
 function that performs transfers. Usage of the \fBCURLOPT_PROGRESSFUNCTION\fP
 callback is not recommended when using the multi interface.
 
-This callback gets called only if you set \fICURLOPT_NOPROGRESS\fP to FALSE.
-
+\fICURLOPT_NOPROGRESS\fP must be set to FALSE to make this function actually
+get called.
 .IP CURLOPT_PROGRESSDATA
 Pass a pointer that will be untouched by libcurl and passed as the first
 argument in the progress callback set with \fICURLOPT_PROGRESSFUNCTION\fP.
-
 .IP CURLOPT_HEADERFUNCTION
 Function pointer that should match the following prototype: \fIsize_t
 function( void *ptr, size_t size, size_t nmemb, void *stream);\fP. This
diff --git a/lib/progress.c b/lib/progress.c
index 65bd4a7bb..5ba829a03 100644
--- a/lib/progress.c
+++ b/lib/progress.c
@@ -280,6 +280,9 @@ 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
@@ -356,10 +359,6 @@ 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 a1e149a12..4f8878401 100644
--- a/lib/select.c
+++ b/lib/select.c
@@ -32,7 +32,9 @@
 #include <sys/time.h>
 #endif
 
-#include <signal.h>
+#ifndef HAVE_SELECT
+#error "We can't compile without select() support!"
+#endif
 
 #ifdef __BEOS__
 /* BeOS has FD_SET defined in socket.h */
@@ -51,57 +53,6 @@
 
 /* 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
@@ -115,56 +66,18 @@
 #endif
 
 /*
- * 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.
- *
- * We return 1 if as a result of the unblocking, a signal was
- * received, caught and handled.  0 otherwise.
+ * 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.
  *
- * On a system that does not have pselect(), we always return 0, even if
- * signals were received.
+ * Return values:
+ *   -1 = system call error
+ *    0 = timeout
+ *    CSELECT_IN | CSELECT_OUT | CSELECT_ERR
  */
-int receive_signals(sigset_t * sigmask)
+int Curl_select(curl_socket_t readfd, curl_socket_t writefd, int timeout_ms)
 {
-#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;
@@ -182,11 +95,13 @@ static int select_with_poll(curl_socket_t readfd, curl_socket_t writefd,
     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;
@@ -217,13 +132,7 @@ static int select_with_poll(curl_socket_t readfd, curl_socket_t writefd,
   }
 
   return ret;
-}
-
-#endif USE_POLL_FOR_SELECT
-
-static int select_with_select(curl_socket_t readfd, curl_socket_t writefd,
-                              int timeout_ms)
-{
+#else
   struct timeval timeout;
   fd_set fds_read;
   fd_set fds_write;
@@ -270,7 +179,9 @@ static int select_with_select(curl_socket_t readfd, curl_socket_t writefd,
       maxfd = writefd;
   }
 
-  r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, &timeout);
+  do {
+    r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, &timeout);
+  } while((r == -1) && (SOCKERRNO == EINTR));
 
   if (r < 0)
     return -1;
@@ -292,49 +203,7 @@ static int select_with_select(curl_socket_t readfd, curl_socket_t writefd,
   }
 
   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 0941db6df..e0844b1d6 100644
--- a/lib/select.h
+++ b/lib/select.h
@@ -51,9 +51,6 @@ 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 07e135ff1..8e6f5e19f 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1603,6 +1603,8 @@ 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 */
@@ -1752,18 +1754,6 @@ int Curl_single_getsock(struct connectdata *conn,
 }
 
 
-
-static bool
-errnoIsInterruption(int errnoarg)
-{
-#ifdef EINTR
-  return (errnoarg == EINTR);
-#else
-  return FALSE;
-#endif
-}
-
-
 /*
  * Transfer()
  *
@@ -1785,12 +1775,6 @@ 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
@@ -1844,33 +1828,28 @@ 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. */
 
-    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 */
+    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 */
       continue;
-    } else {
-      /* ready files, timeout, or signal received */
+    case 0:  /* timeout */
+    default: /* readable descriptors */
 
       result = Curl_readwrite(conn, &done);
-
-      /* "done" signals to us if the transfer(s) are ready */
-
-      if(result)
-        return result;
+      break;
     }
+    if(result)
+      return result;
+
+    /* "done" signals to us if the transfer(s) are ready */
   }
-  Curl_pgrsUpdate(conn);
 
   return CURLE_OK;
 }