retrying a Curl::Easy about curb HOT 10 CLOSED

Why do you have the m.requests.empty? loop around the m.perform? Removing that and I get no errors... I never thought about using m.requests like that... Looking into a patch now to prevent that error...

from curb.

That's the work-around for problem number 2. If you remove the m.requests.empty? loop the request is never run a second time.

from curb.

Okay, it took me a little bit to understand this use case. Using your first suggestion, storing a local reference to self - did the trick. I pushed an updated version of the code curb-0.5.7

from curb.

Awesome, thanks!

For the second issue, I was thinking of something like this:

--- a/ext/curb_multi.c
+++ b/ext/curb_multi.c
@@ -214,9 +214,9 @@ VALUE ruby_curl_multi_add(VALUE self, VALUE easy) {
   ruby_curl_easy_setup( rbce, &(rbce->bodybuf), &(rbce->headerbuf), &(rbce->curl_headers) );

   rbcm->active++;
-  if (mcode == CURLM_CALL_MULTI_PERFORM) {
-    curl_multi_perform(rbcm->handle, &(rbcm->running));
-  }
+
+  /* call curl_multi_perform either way to update rbcm->running */
+  curl_multi_perform(rbcm->handle, &(rbcm->running));

   rb_hash_aset( rbcm->requests, easy, easy );
   // active should equal INT2FIX(RHASH(rbcm->requests)->tbl->num_entries)

Everything seems to work properly as long as rbcm->running is updated to show that there is a new handle to process. I'm not sure why libcurl doesn't return CURLM_CALL_MULTI_PERFORM in this case, but I think it probably should. Anyway, it seems safe to call curl_multi_perform either way.

from curb.

Oh, reading the libcurl source and docs, it looks like CURLM_CALL_MULTI_PERFORM is not a valid return value of curl_multi_add_handle(), so that check was never true. Even more of a reason to make this change :)

from curb.

Looking at the docs now, I don't see it saying that is not a valid return value here: http://curl.haxx.se/libcurl/c/curl_multi_add_handle.html and here http://curl.haxx.se/libcurl/c/curl_multi_perform.html it says explicitly that you should call curl_multi_perform if CURLM_CALL_MULTI_PERFORM is returned. I think maybe that second reference is the source of my confusion? It is the return value of curl_multi_perform not curl_multi_add_handle. Okay now I understand that. Okay so now the question is do we wan the request to start on multi.add or on mult.perform? I would think for example:

10.times do
  m.add(Curl::Easy.new($TEST_URL))
end

assert_equal(10, m.requests.length, 'multi.requests should contain all the active requests')

m.perform

We would want to perform not on add but on perform....

Let me know what you think... I can adjust that test case, although I think for ruby at least it's a little bit easier to understand if it's a request on perform instead of request on add...

from curb.

Yeah, I don't think adding should actually start performing. The goal is just to get rbcm->running > 0 in case add() is being called from inside the perform() loop (in an event handler). Maybe just manually increasing rbcm->running would work, so that the loop would run and the next call to curl_multi_perform() would correct it?

from curb.

Okay, I think I have a solution. This calling semantics you would like make sense in your use case, so I believe to resolve this you can pass a flag to multi.add(easy,true) . That will tell the multi handle to call perform immediately which is what you want in your case. The default case would still be to not call perform immediately.... I'm committing the changes in just a few, if you agree this change is right.

from curb.

No, I don't need it to perform immediately, just keep running the perform loop if it's already running (and not perform if it is not running, which was the bug with my previous diff). I think this diff summarizes it:

diff --git a/ext/curb_multi.c b/ext/curb_multi.c
index 3754eca..51a392c 100644
--- a/ext/curb_multi.c
+++ b/ext/curb_multi.c
@@ -214,9 +214,10 @@ VALUE ruby_curl_multi_add(VALUE self, VALUE easy) {
   ruby_curl_easy_setup( rbce, &(rbce->bodybuf), &(rbce->headerbuf), &(rbce->curl_headers) );

   rbcm->active++;
-  if (mcode == CURLM_CALL_MULTI_PERFORM) {
-    curl_multi_perform(rbcm->handle, &(rbcm->running));
-  }
+
+  /* Increase the running count, so that the perform loop keeps running.
+     If this number is not correct, the next call to curl_multi_perform will correct it. */
+  rbcm->running++;

   rb_hash_aset( rbcm->requests, easy, easy );
   // active should equal INT2FIX(RHASH(rbcm->requests)->tbl->num_entries)

from curb.

That is a better solution. Also, I think the call to rb_curl_multi_read_info is unnecessary as well....

from curb.