Guarantee that curl_multi_remove_handle is called #423
Conversation
Add a drop handler that guarantees that `curl_multi_remove_handle` is always called on easy handles added to a multi handle before the easy handle is dropped. Based on curl/curl#7982, we should not be allowing users to drop a previously attached handle without first calling `curl_multi_remove_handle`. In curl versions between 7.77 and 7.80 this could cause a crash under certain circumstances. While this will likely be fixed in the next curl version, it is still recommended to not allow this, plus we must still handle this case for users who may not have updated to a patched libcurl. I'm not totally happy with how this is implemented as it adds an `Arc::clone` call every time an easy handle is added to a multi handle, but I couldn't think of a better way of guaranteeing this behavior without breaking API changes. Fixes #421.
|
@iiibui Can you verify that using this branch fixes your crashes even with curl 7.80? It prevented the double-free errors in my tests. |
This branch fixes my origin crashes with curl 7.80, but I found it has no effect with curl commit |
|
@iiibui Hmm yeah, I still get the double-free against
So the only released version with this issue still is 7.77.0, but that version had a known issue around that which was fixed in 7.78.0: curl/curl#7236. |
Add a drop handler that guarantees that
curl_multi_remove_handleis always called on easy handles added to a multi handle before the easy handle is dropped. Based on curl/curl#7982, we should not be allowing users to drop a previously attached handle without first callingcurl_multi_remove_handle. In curl versions between 7.77 and 7.80 this could cause a crash under certain circumstances. While this will likely be fixed in the next curl version, it is still recommended to not allow this, plus we must still handle this case for users who may not have updated to a patched libcurl.I'm not totally happy with how this is implemented as it adds an
Arc::clonecall every time an easy handle is added to a multi handle, but I couldn't think of a better way of guaranteeing this behavior without breaking API changes.I've verified that this does actually fix #421 even without the upstream curl patch.
Fixes #421.