cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Busy looping bug in multi_socket interface

From: Christopher Palow <cpalow_at_facebook.com>
Date: Mon, 05 May 2008 19:42:52 -0700

I donıt know if Yang is being sarcastic or not. If heıs not being sarcastic
how is this patch? I use epoch for both /* splay the lowest to the bottom
*/ and KEY_NOTUSED. Iım not sure how comfortable I am with that but I think
itıs fine.

Thanks,
Chris

Index: docs/libcurl/curl_multi_add_handle.3
===================================================================
RCS file: /cvsroot/curl/curl/docs/libcurl/curl_multi_add_handle.3,v
retrieving revision 1.5
diff -u -w -p -r1.5 curl_multi_add_handle.3
--- docs/libcurl/curl_multi_add_handle.3 7 Jul 2006 23:08:03 -0000
1.5
+++ docs/libcurl/curl_multi_add_handle.3 6 May 2008 02:42:02 -0000
@@ -11,6 +11,8 @@ CURLMcode curl_multi_add_handle(CURLM *m
 .SH DESCRIPTION
 Adds a standard easy handle to the multi stack. This function call will
make
 this \fImulti_handle\fP control the specified \fIeasy_handle\fP.
+Furthermore, libcurl now initiates the connection associated with the
+specified \fIeasy_handle\fP.
 
 When an easy handle has been added to a multi stack, you can not and you
must
 not use \fIcurl_easy_perform(3)\fP on that handle!
Index: docs/libcurl/curl_multi_socket.3
===================================================================
RCS file: /cvsroot/curl/curl/docs/libcurl/curl_multi_socket.3,v
retrieving revision 1.12
diff -u -w -p -r1.12 curl_multi_socket.3
--- docs/libcurl/curl_multi_socket.3 6 Mar 2008 12:43:47 -0000 1.12
+++ docs/libcurl/curl_multi_socket.3 6 May 2008 02:42:03 -0000
@@ -46,9 +46,7 @@ socket callback function set with the CU
 previous time this function was called.
 
 Force libcurl to (re-)check all its internal sockets and transfers instead
of
-just a single one by calling \fBcurl_multi_socket_all(3)\fP. This is
typically
-done as the first function call before the application has any knowledge
about
-what sockets libcurl uses.
+just a single one by calling \fBcurl_multi_socket_all(3)\fP.
 
 Get the timeout time - how long to wait for socket actions at most before
 doing the timeout action: call the \fBcurl_multi_socket(3)\fP function with
@@ -125,22 +123,20 @@ function returns OK.
 3. Set the timeout callback with CURLMOPT_TIMERFUNCTION, to get to know
what
 timeout value to use when waiting for socket activities.
 
-4. Add easy handles
+4. Add easy handles with curl_multi_add_handle()
 
-5. Call curl_multi_socket_all() first once
-
-6. Provide some means to manage the sockets libcurl is using, so you can
check
+5. Provide some means to manage the sockets libcurl is using, so you can
check
 them for activity. This can be done through your application code, or by
way
 of an external library such as libevent or glib.
 
-7. Wait for activity on any of libcurl's sockets, use the timeout value
your
+6. Wait for activity on any of libcurl's sockets, use the timeout value
your
 callback has been told
 
-8, When activity is detected, call curl_multi_socket_action() for the
+7, When activity is detected, call curl_multi_socket_action() for the
 socket(s) that got action. If no activity is detected and the timeout
expires,
 call \fIcurl_multi_socket(3)\fP with \fICURL_SOCKET_TIMEOUT\fP
 
-9. Go back to step 7.
+8. Go back to step 6.
 .SH AVAILABILITY
 This function was added in libcurl 7.15.4, although deemed stable since
 7.16.0.
Index: lib/multi.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/multi.c,v
retrieving revision 1.168
diff -u -w -p -r1.168 multi.c
--- lib/multi.c 30 Apr 2008 21:20:09 -0000 1.168
+++ lib/multi.c 6 May 2008 02:42:03 -0000
@@ -177,7 +177,7 @@ struct Curl_multi {
   /* timer callback and user data pointer for the *socket() API */
   curl_multi_timer_callback timer_cb;
   void *timer_userp;
- time_t timer_lastcall; /* the fixed time for the timeout for the previous
+ struct timeval timer_lastcall; /* the fixed time for the timeout for the
previous
                             callback */
 };
 
@@ -1446,9 +1446,8 @@ CURLMcode curl_multi_perform(CURLM *mult
    */
   do {
     struct timeval now = Curl_tvnow();
- int key = now.tv_sec; /* drop the usec part */
 
- multi->timetree = Curl_splaygetbest(key, multi->timetree, &t);
+ multi->timetree = Curl_splaygetbest(now, multi->timetree, &t);
     if(t) {
       struct SessionHandle *d = t->payload;
       struct timeval* tv = &d->state.expiretime;
@@ -1746,7 +1745,6 @@ static CURLMcode multi_socket(struct Cur
    * handle we deal with.
    */
   do {
- int key;
     struct timeval now;
 
     /* the first loop lap 'data' can be NULL */
@@ -1763,9 +1761,8 @@ static CURLMcode multi_socket(struct Cur
        extracts a matching node if there is one */
 
     now = Curl_tvnow();
- key = now.tv_sec; /* drop the usec part */
 
- multi->timetree = Curl_splaygetbest(key, multi->timetree, &t);
+ multi->timetree = Curl_splaygetbest(now, multi->timetree, &t);
     if(t) {
       /* assign 'data' to be the easy handle we just removed from the splay
          tree */
@@ -1855,25 +1852,43 @@ CURLMcode curl_multi_socket_all(CURLM *m
   return result;
 }
 
-static CURLMcode multi_timeout(struct Curl_multi *multi,
- long *timeout_ms)
+static CURLMcode multi_timeout_usec(struct Curl_multi *multi,
+ long *timeout_us)
 {
   if(multi->timetree) {
     /* we have a tree of expire times */
     struct timeval now = Curl_tvnow();
+ struct timeval diff = {0, 0};
 
     /* splay the lowest to the bottom */
- multi->timetree = Curl_splay(0, multi->timetree);
+ multi->timetree = Curl_splay(diff, multi->timetree);
 
- /* At least currently, the splay key is a time_t for the expire time */
- *timeout_ms = (multi->timetree->key - now.tv_sec) * 1000 -
- now.tv_usec/1000;
- if(*timeout_ms < 0)
+ /* At least currently, the splay key is the struct timeval of the
expire time */
+ timersub(&multi->timetree->key, &now, &diff);
+ *timeout_us = diff.tv_sec * 1000000 + diff.tv_usec;
+
+ if(*timeout_us < 0)
       /* 0 means immediately */
- *timeout_ms = 0;
+ *timeout_us = 0;
   }
   else
- *timeout_ms = -1;
+ *timeout_us = -1;
+
+ return CURLM_OK;
+}
+
+
+static CURLMcode multi_timeout(struct Curl_multi *multi,
+ long *timeout_ms)
+{
+ long timeout_us;
+
+ multi_timeout_usec(multi, &timeout_us);
+
+ if(timeout_us > 0) /* convert to milliseconds */
+ *timeout_ms=timeout_us/1000;
+ else /* immediately or error condition */
+ *timeout_ms=timeout_us;
 
   return CURLM_OK;
 }
@@ -1908,7 +1923,7 @@ static int update_timer(struct Curl_mult
    * timeout we got the (relative) time-out time for. We can thus easily
check
    * if this is the same (fixed) time as we got in a previous call and then
    * avoid calling the callback again. */
- if(multi->timetree->key == multi->timer_lastcall)
+ if(timercmp(&multi->timetree->key, &multi->timer_lastcall, ==))
     return 0;
 
   multi->timer_lastcall = multi->timetree->key;
@@ -2054,8 +2069,7 @@ void Curl_expire(struct SessionHandle *d
           (long)nowp->tv_sec, (long)nowp->tv_usec, milli);
 #endif
     data->state.timenode.payload = data;
- multi->timetree = Curl_splayinsert((int)nowp->tv_sec,
- multi->timetree,
+ multi->timetree = Curl_splayinsert(*nowp, multi->timetree,
                                        &data->state.timenode);
   }
 #if 0
Index: lib/splay.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/splay.c,v
retrieving revision 1.8
diff -u -w -p -r1.8 splay.c
--- lib/splay.c 5 Nov 2007 09:45:09 -0000 1.8
+++ lib/splay.c 6 May 2008 02:42:04 -0000
@@ -28,19 +28,20 @@
 
 #include "splay.h"
 
-#define compare(i,j) ((i)-(j))
+#define compare(i,j) timercmp(&(i), &(j), -)
 
+struct timeval key_notused_epoch= {0, 0};
 /* Set this to a key value that will *NEVER* appear otherwise */
-#define KEY_NOTUSED -1
+#define KEY_NOTUSED key_notused_epoch
 
 /*
  * Splay using the key i (which may or may not be in the tree.) The
starting
  * root is t.
  */
-struct Curl_tree *Curl_splay(int i, struct Curl_tree *t)
+struct Curl_tree *Curl_splay(struct timeval i, struct Curl_tree *t)
 {
   struct Curl_tree N, *l, *r, *y;
- int comp;
+ time_t comp;
 
   if(t == NULL)
     return t;
@@ -93,7 +94,7 @@ struct Curl_tree *Curl_splay(int i, stru
 
 /* Insert key i into the tree t. Return a pointer to the resulting tree or
    NULL if something went wrong. */
-struct Curl_tree *Curl_splayinsert(int i,
+struct Curl_tree *Curl_splayinsert(struct timeval i,
                                    struct Curl_tree *t,
                                    struct Curl_tree *node)
 {
@@ -149,7 +150,7 @@ struct Curl_tree *Curl_splayinsert(int i
 
    Function not used in libcurl.
 */
-struct Curl_tree *Curl_splayremove(int i, struct Curl_tree *t,
+struct Curl_tree *Curl_splayremove(struct timeval i, struct Curl_tree *t,
                                    struct Curl_tree **removed)
 {
   struct Curl_tree *x;
@@ -194,7 +195,7 @@ struct Curl_tree *Curl_splayremove(int i
 
 /* Finds and deletes the best-fit node from the tree. Return a pointer to
the
    resulting tree. best-fit means the node with the given or lower number
*/
-struct Curl_tree *Curl_splaygetbest(int i, struct Curl_tree *t,
+struct Curl_tree *Curl_splaygetbest(struct timeval i, struct Curl_tree *t,
                                     struct Curl_tree **removed)
 {
   struct Curl_tree *x;
@@ -268,7 +269,7 @@ int Curl_splayremovebyaddr(struct Curl_t
   if(!t || !removenode)
     return 1;
 
- if(KEY_NOTUSED == removenode->key) {
+ if(timercmp(&KEY_NOTUSED,&removenode->key, ==)) {
     /* Key set to NOTUSED means it is a subnode within a 'same' linked list
        and thus we can unlink it easily. The 'smaller' link of a subnode
        links to the parent node. */
@@ -341,7 +342,7 @@ void Curl_splayprint(struct Curl_tree *
       printf(" ");
 
   if(output) {
- printf("%d[%d]", t->key, i);
+ printf("%lu[%d]", (long)t->key.tv_usec, i);
   }
 
   for(count=0, node = t->same; node; node = node->same, count++)
Index: lib/splay.h
===================================================================
RCS file: /cvsroot/curl/curl/lib/splay.h,v
retrieving revision 1.4
diff -u -w -p -r1.4 splay.h
--- lib/splay.h 27 Sep 2007 01:45:23 -0000 1.4
+++ lib/splay.h 6 May 2008 02:42:04 -0000
@@ -27,19 +27,19 @@ struct Curl_tree {
   struct Curl_tree *smaller; /* smaller node */
   struct Curl_tree *larger; /* larger node */
   struct Curl_tree *same; /* points to a node with identical key */
- int key; /* the "sort" key */
+ struct timeval key; /* the "sort" key */
   void *payload; /* data the splay code doesn't care about */
 };
 
-struct Curl_tree *Curl_splay(int i, struct Curl_tree *t);
-struct Curl_tree *Curl_splayinsert(int key, struct Curl_tree *t,
+struct Curl_tree *Curl_splay(struct timeval i, struct Curl_tree *t);
+struct Curl_tree *Curl_splayinsert(struct timeval key, struct Curl_tree *t,
                                    struct Curl_tree *newnode);
 #if 0
-struct Curl_tree *Curl_splayremove(int key, struct Curl_tree *t,
+struct Curl_tree *Curl_splayremove(struct timeval key, struct Curl_tree *t,
                                    struct Curl_tree **removed);
 #endif
 
-struct Curl_tree *Curl_splaygetbest(int key, struct Curl_tree *t,
+struct Curl_tree *Curl_splaygetbest(struct timeval key, struct Curl_tree
*t,
                                     struct Curl_tree **removed);
 int Curl_splayremovebyaddr(struct Curl_tree *t,
                            struct Curl_tree *removenode,

On 4/30/08 4:42 PM, "Yang Tse" <yangsita_at_gmail.com> wrote:

> Yes, we don't need K&R compatibility, C89 is enough!
Received on 2008-05-06