[RFC] are the curl_multi::get_info and curl_multi::is_finished methods broken?

[dvd0101]

IIUC the documentation every messages are returned only once and then removed from the internal handle.

But curl_multi::is_finished consumes all the queue searching for a specific easy handle, discarding the messages it is not interested in.

auto m = curl::curl_multi{};

m.add(easy1);
m.add(easy2);

// do something

std::cout << m.is_finished(easy1) << std::endl;
std::cout << m.is_finished(easy2) << std::endl;

The first call to curl_multi::is_finished could discard a message for easy2.

What if we add an unordered_map[easy_handle->message] in the curl_multi class in order to store the messages returned by curl_multi_info_read (overwrite the old ones should be ok)?

According to this answer on stackoverflow in order to reuse an easy handle you have to remove it, change and then re-add to the multi instance; if so we can cleanup the messages map during the remove phase.

[dvd0101]

if you don't have any major objections I can try to submit a patch in the next days

[JosephP91]

Sure you can! @cinghiale

[JosephP91]

@cinghiale any news about the patch? :)

[czaarek99]

@JosephP91 I don't think he's coming back to fix this xd

[JosephP91]

@czaarek99 I hope nothing bad happened to him!

[desertkun]

Seems like I have been ran into this exact issue, and it is way too critical.

To test this, I've made a server script that responds in 5s, and then added 100 requests into the curl_multi.

I'm updating requests in that manner:

m_requests – a list of my wrappers around curl_multi, and m_transport – curl_multi

        m_requests.remove_if([=](RequestPtr &it) -> bool {
             if (m_transport.is_finished(it->getTransport())) {
                 it->done();
                 m_transport.remove(it->getTransport());
                 return true;
             }
             return false;
         });

In most cases I only get 5 respons out of 100. And server gives valid 100 responses!

Seems like it's a bad design to check requests like this and I need to use curl_multi_info_read directly, but yet, I can confirm that this issue is 100% true.

[desertkun]

I've added a method get_next_finished so my issue can be fixed like this:

while ((next = m_transport.get_next_finished())) {
    std::unordered_map<curl::curl_easy*, RequestPtr>::const_iterator it = m_requests.find(next);
    if (it != m_requests.end()) {
        const RequestPtr& request = it->second;
        request->done();
        m_transport.remove(*next);
        m_requests.erase(it);
    }
}

100 out of 100 is received.

[JosephP91]

@desertkun I've enriched the get_next_finished. Now it returns a complete message with the related curl_easy object.
@dvd0101 I've removed the broken methods: get_info and is_finished