Commit 840e796a authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

Sebastien Willemijns reported bug #1603712

(http://curl.haxx.se/bug/view.cgi?id=1603712) which is about connections
getting cut off prematurely when --limit-rate is used. While I found no such
problems in my tests nor in my reading of the code, I found that the
--limit-rate code was severly flawed (since it was moved into the lib, since
7.15.5) when used with the easy interface and it didn't work as documented so
I reworked it somewhat and now it works for my tests.
parent 5fd096da
Loading
Loading
Loading
Loading
+9 −0
Original line number Diff line number Diff line
@@ -6,6 +6,15 @@

                                  Changelog

Daniel (6 December 2006)
- Sebastien Willemijns reported bug #1603712
  (http://curl.haxx.se/bug/view.cgi?id=1603712) which is about connections
  getting cut off prematurely when --limit-rate is used. While I found no such
  problems in my tests nor in my reading of the code, I found that the
  --limit-rate code was severly flawed (since it was moved into the lib, since
  7.15.5) when used with the easy interface and it didn't work as documented
  so I reworked it somewhat and now it works for my tests.

Daniel (5 December 2006)
- Stefan Krause pointed out a compiler warning with a picky MSCV compiler when
  passing a curl_off_t argument to the Curl_read_rewind() function which takes
+2 −1
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ This release includes the following bugfixes:
 o curl_getdate() could be off one hour for TZ time zones with DST, on windows
 o CURLOPT_FORBID_REUSE works again
 o CURLOPT_MAXCONNECTS set to zero caused libcurl to SIGSEGV
 o rate limiting works better

Other curl-related news:

@@ -50,6 +51,6 @@ advice from friends like these:
 James Housley, Olaf Stueben, Yang Tse, Gisle Vanem, Bradford Bruce,
 Ciprian Badescu, Dmitriy Sergeyev, Nir Soffer, Venkat Akella, Toon Verwaest,
 Matt Witherspoon, Alexey Simak, Martin Skinner, Sh Diao, Jared Lundell,
 Stefan Krause
 Stefan Krause, Sebastien Willemijns

        Thanks! (and sorry if I forgot to mention someone)
+0 −3
Original line number Diff line number Diff line
@@ -3,9 +3,6 @@ To get fixed in 7.16.1 (planned release: January 2007)

69 - Jeff Pohlmeyer's crashing case (posted 17 nov 2006)

71 - rate-limit when transferring files (using the easy interface) is
     completely broken, bug #1603712. Patch pending commit on feedback.

75 - "copying everything downloaded" (posted 11 nov 2006).

77 -
+28 −22
Original line number Diff line number Diff line
@@ -311,12 +311,15 @@ CURLcode Curl_readwrite(struct connectdata *conn,

  curl_off_t contentlength;

  if(k->keepon & KEEP_READ)
  /* only use the proper socket if the *_HOLD bit is not set simultaneously as
     then we are in rate limiting state in that transfer direction */

  if((k->keepon & (KEEP_READ|KEEP_READ_HOLD)) == KEEP_READ)
    fd_read = conn->sockfd;
  else
    fd_read = CURL_SOCKET_BAD;

  if(k->keepon & KEEP_WRITE)
  if((k->keepon & (KEEP_WRITE|KEEP_WRITE_HOLD)) == KEEP_WRITE)
    fd_write = conn->writesockfd;
  else
    fd_write = CURL_SOCKET_BAD;
@@ -1530,7 +1533,7 @@ CURLcode Curl_readwrite(struct connectdata *conn,
  }

  /* Now update the "done" boolean we return */
  *done = (bool)(0 == k->keepon);
  *done = (bool)(0 == (k->keepon&(KEEP_READ|KEEP_WRITE)));

  return CURLE_OK;
}
@@ -1699,37 +1702,40 @@ Transfer(struct connectdata *conn)
  while (!done) {
    curl_socket_t fd_read;
    curl_socket_t fd_write;
    int interval_ms;

    interval_ms = 1 * 1000;

    /* limit-rate logic: if speed exceeds threshold, then do not include fd in
       select set */
    if ( (data->set.max_send_speed > 0) &&
         (data->progress.ulspeed > data->set.max_send_speed) )  {
      fd_write = CURL_SOCKET_BAD;
      Curl_pgrsUpdate(conn);
       select set. The current speed is recalculated in each Curl_readwrite()
       call */
    if ((k->keepon & KEEP_WRITE) &&
        (!data->set.max_send_speed ||
         (data->progress.ulspeed < data->set.max_send_speed) )) {
      fd_write = conn->writesockfd;
      k->keepon &= ~KEEP_WRITE_HOLD;
    }
    else {
      if(k->keepon & KEEP_WRITE)
        fd_write = conn->writesockfd;
      else
      fd_write = CURL_SOCKET_BAD;
      if(k->keepon & KEEP_WRITE)
        k->keepon |= KEEP_WRITE_HOLD; /* hold it */
    }

    if ( (data->set.max_recv_speed > 0) &&
         (data->progress.dlspeed > data->set.max_recv_speed) ) {
      fd_read = CURL_SOCKET_BAD;
      Curl_pgrsUpdate(conn);
    if ((k->keepon & KEEP_READ) &&
        (!data->set.max_recv_speed ||
         (data->progress.dlspeed < data->set.max_recv_speed)) ) {
      fd_read = conn->sockfd;
      k->keepon &= ~KEEP_READ_HOLD;
    }
    else {
      if(k->keepon & KEEP_READ)
        fd_read = conn->sockfd;
      else
      fd_read = CURL_SOCKET_BAD;
      if(k->keepon & KEEP_READ)
        k->keepon |= KEEP_READ_HOLD; /* hold it */
    }

    switch (Curl_select(fd_read, fd_write, interval_ms)) {
    /* The *_HOLD logic is necessary since even though there might be no
       traffic during the select interval, we still call Curl_readwrite() for
       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
+7 −5
Original line number Diff line number Diff line
@@ -503,12 +503,14 @@ struct hostname {
/*
 * Flags on the keepon member of the Curl_transfer_keeper
 */
enum {
  KEEP_NONE,
  KEEP_READ,
  KEEP_WRITE
};

#define KEEP_NONE  0
#define KEEP_READ  1      /* there is or may be data to read */
#define KEEP_WRITE 2      /* there is or may be data to write */
#define KEEP_READ_HOLD 4  /* when set, no reading should be done but there
                             might still be data to read */
#define KEEP_WRITE_HOLD 8 /* when set, no writing should be done but there
                             might still be data to write */

/*
 * This struct is all the previously local variables from Curl_perform() moved