Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

examples/10-at-a-time: fix possible skipped final transfers #9954

Closed
wants to merge 2 commits into from

Conversation

jay
Copy link
Member

@jay jay commented Nov 21, 2022

Prior to this change if curl_multi_perform returned 0 running handles and then all remaining transfers were added, then the perform loop would end immediately without performing those transfers.

Reported-by: Mikhail Kuznetsov

Fixes #9953
Closes #xxxx

Prior to this change if curl_multi_perform returned 0 running handles
and then all remaining transfers were added, then the perform loop would
end immediately without performing those transfers.

Reported-by: Mikhail Kuznetsov

Fixes curl#9953
Closes #xxxx
@bagder
Copy link
Member

bagder commented Nov 21, 2022

Alternative fix:

diff --git a/docs/examples/10-at-a-time.c b/docs/examples/10-at-a-time.c
index 1739a9e78..40ec52cd0 100644
--- a/docs/examples/10-at-a-time.c
+++ b/docs/examples/10-at-a-time.c
@@ -93,37 +93,39 @@ static size_t write_cb(char *data, size_t n, size_t l, void *userp)
   (void)data;
   (void)userp;
   return n*l;
 }
 
-static void add_transfer(CURLM *cm, int i)
+static void add_transfer(CURLM *cm, int i, int *left)
 {
   CURL *eh = curl_easy_init();
   curl_easy_setopt(eh, CURLOPT_WRITEFUNCTION, write_cb);
   curl_easy_setopt(eh, CURLOPT_URL, urls[i]);
   curl_easy_setopt(eh, CURLOPT_PRIVATE, urls[i]);
   curl_multi_add_handle(cm, eh);
+  (*left)++;
 }
 
 int main(void)
 {
   CURLM *cm;
   CURLMsg *msg;
   unsigned int transfers = 0;
   int msgs_left = -1;
-  int still_alive = 1;
+  int left = 0;
 
   curl_global_init(CURL_GLOBAL_ALL);
   cm = curl_multi_init();
 
   /* Limit the amount of simultaneous connections curl should allow: */
   curl_multi_setopt(cm, CURLMOPT_MAXCONNECTS, (long)MAX_PARALLEL);
 
   for(transfers = 0; transfers < MAX_PARALLEL; transfers++)
-    add_transfer(cm, transfers);
+    add_transfer(cm, transfers, &left);
 
   do {
+    int still_alive = 1;
     curl_multi_perform(cm, &still_alive);
 
     while((msg = curl_multi_info_read(cm, &msgs_left))) {
       if(msg->msg == CURLMSG_DONE) {
         char *url;
@@ -131,21 +133,22 @@ int main(void)
         curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE, &url);
         fprintf(stderr, "R: %d - %s <%s>\n",
                 msg->data.result, curl_easy_strerror(msg->data.result), url);
         curl_multi_remove_handle(cm, e);
         curl_easy_cleanup(e);
+        left--;
       }
       else {
         fprintf(stderr, "E: CURLMsg (%d)\n", msg->msg);
       }
       if(transfers < NUM_URLS)
-        add_transfer(cm, transfers++);
+        add_transfer(cm, transfers++, &left);
     }
-    if(still_alive)
+    if(left)
       curl_multi_wait(cm, NULL, 0, 1000, NULL);
 
-  } while(still_alive || (transfers < NUM_URLS));
+  } while(left);
 
   curl_multi_cleanup(cm);
   curl_global_cleanup();
 
   return EXIT_SUCCESS;

@kuznetsov-m
Copy link

Yes, it works! Alternative fix also looks great!

@jay jay closed this in a28a80d Nov 22, 2022
@jay
Copy link
Member Author

jay commented Nov 22, 2022

Alternative fix

Sure that works as well and fixes it in half the lines that I had changed. I think that the initial loop to add transfers should still be conditional on transfers < MAX_PARALLEL && transfers < NUM_URLS so I've added that.

Yes, it works! Alternative fix also looks great!

Thanks for the report and testing.

@jay jay deleted the fix_10-at-a-time branch November 22, 2022 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

10-at-a-time example skips result of the last url in single MAX_PARALLEL mode
3 participants