Critical:: Unable to remove easy from requests

[taf2]

Critical:: Unable to remove easy from requests

[ghazel]

Looking at the logs, I can tell you there was a long-lived Curl::Multi handle which had never had any Easy handles re-added.

[taf2]

Can you try again with 0.5.8.0 ?

[taf2]

You know it might be that...
if( r != easy || r == Qnil ) {
rb_raise(rb_eRuntimeError, "Critical:: Unable to remove easy from requests");
}

should really be
if( r != easy && r == Qnil ) {
rb_raise(rb_eRuntimeError, "Critical:: Unable to remove easy from requests");
}

Still thinking about that branch...

[taf2]

Actually you know what, that error could be coming from 2 places. I believe curl_multi_flush_easy does not need to remove items from the hash, since it's called within a foreach loop, I wonder if removing items within a ruby foreach is save in the C API... I think we can skip that code since we dereference the Hash following that loop...

[taf2]

Can you run your test against my last commit http://github.com/taf2/curb/commit/f97799351f84a8f050d4478e2f94cded5f3f9fb8

[ghazel]

Running with that change. Will reopen or file a new ticket if I have any other problems, but with that exception gone I might not get it again...

[ghazel]

Btw, judging from the code, modifying a hash during rb_hash_foreach is not safe.

[taf2]

did the change fix your bug?

[ghazel]

Well, I haven't seen the exception and it's been running for 15 hours. So my guess is it's fixed..

[ghazel]

Spoke too soon. Just got this error again with 0.5.9.1.

[taf2]

can't be the same error what was the message or stack trace?

[ghazel]

I just got "Critical:: Unable to remove easy from requests" inside a Curl::Mutli perform. Not sure what the C stack was.

[taf2]

internally, the multi handle uses the address of that easy handle as a reference within the requests Hash to prevent the ruby GC from collecting any active easy requests... I think if we check for the handle in the add and remove it if found then on handle complete when it's time to remove the request we wont' get that exception raised. Going to work on this a bit today and will let you know when I have a patch for you test again. Thanks !

[taf2]

For the time being I changed things to just warn, instead of raising in this condition, it should be fairly safe... e.g. at worst a memory leak...

[ghazel]

If you make it printf when an easy handle is added and deleted (and why it's being deleted), and still raise the exception when the delete fails, I could you provide you with the log. It's either removing twice or removing something which is never added, right?

I've encountered about 7 of these so far, so a leak wouldn't be so bad I guess, but I'd like to help find/fix the problem.

[taf2]

I've reworked a few things in how the multi handle interacts with the easy handle. I'm wondering can you clone from HEAD and check in your environment?

[ghazel]

Sorry for the delay. Running HEAD now, will report back in a few days.

[taf2]

any luck?

[ghazel]

Well, it's still running and I haven't had any problems. I don't think there have been any crashes, and I haven't seen any memory leaks, but I restart my processes every 24 hours.

Is there anything else I can check to be sure?

[taf2]

I think that is good, as I recall I had pretty much removed the code path that could have been causing the issue to begin with... I'm going to close the issue out.

[ghazel]

Cool.