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

Jump to bottom

AsynchDNS errors are not handled correctly in multi in 7.65 #4144

Closed
ngg opened this issue Jul 23, 2019 · 2 comments
Closed

AsynchDNS errors are not handled correctly in multi in 7.65 #4144

ngg opened this issue Jul 23, 2019 · 2 comments
Assignees
@bagder
Labels
name lookup DNS and related tech

Comments

@ngg
Copy link
Contributor

ngg commented Jul 23, 2019

This issue is split off from #4135 as I think it is a distinct issue (probably caused by the same underlying bug).

Since #3651 has been merged, errors during asynchronous IP resolution are not handled correctly when using the multi interface, resulting in hanging connections.

In the following example, we have two transfers to the same non-existing host, and the multi is configured to run maximum 1 connection parallel, this way the second transfer tries to reuse the first connection and hangs. Before #3651 the first connection was removed from the connection cache after an error, and the second transfer created a new connection. After that patch, somehow the first connection stays in the cache and the second transfer waits for it indefinitely because it thinks "Connection #0 is still name resolving".

If I disable the threaded resolver, it works ok.

#include <curl/curl.h>
#include <stdio.h>

static int debug_callback(CURL* handle, curl_infotype type, char* data, size_t size, void* userptr)
{
    switch (type) {
    case CURLINFO_TEXT:
        fprintf(stderr, "== Info (%d): %s", (int)userptr, data);
        break;
    }
    return 0;
}

int main(void)
{
    curl_global_init(CURL_GLOBAL_DEFAULT);

    CURLM* multi_handle = curl_multi_init();
    curl_multi_setopt(multi_handle, CURLMOPT_MAX_TOTAL_CONNECTIONS, 1L);

    for (int i = 0; i < 2; i++) {
        CURL* http_handle = curl_easy_init();
        curl_easy_setopt(http_handle, CURLOPT_URL, "http://non-existing-host.haxx.se");
        curl_easy_setopt(http_handle, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);
        curl_easy_setopt(http_handle, CURLOPT_DEBUGFUNCTION, debug_callback);
        curl_easy_setopt(http_handle, CURLOPT_DEBUGDATA, (void*)i);
        curl_easy_setopt(http_handle, CURLOPT_VERBOSE, 1L);
        curl_multi_add_handle(multi_handle, http_handle);
    }

    int still_running, last = 0;
    do {
        CURLMcode mc = curl_multi_perform(multi_handle, &still_running);
        if (mc != CURLM_OK) {
            fprintf(stderr, "curl_multi_perform() failed, code %d.\n", mc);
            return 1;
        }
        if (still_running != last) {
            fprintf(stderr, "still_running = %d\n", still_running);
            last = still_running;
        }

        int numfds;
        mc = curl_multi_wait(multi_handle, NULL, 0, 1000, &numfds);
        if (mc != CURLM_OK) {
            fprintf(stderr, "curl_multi_wait() failed, code %d.\n", mc);
            return 1;
        }
    } while (still_running);
}

Current output (it hangs in that state):

== Info (0): STATE: INIT => CONNECT handle 0x5627fd09ea98; line 1348 (connection #-5000)
== Info (0): Added connection 0. The cache now contains 1 members
== Info (0): STATE: CONNECT => WAITRESOLVE handle 0x5627fd09ea98; line 1384 (connection #0)
== Info (1): STATE: INIT => CONNECT handle 0x5627fd0a4348; line 1348 (connection #-5000)
== Info (1): Found bundle for host non-existing-host.haxx.se: 0x5627fd0753f8 [serially]
== Info (1): No connections available in cache
== Info (1): No connections available.
== Info (1): STATE: CONNECT => CONNECT_PEND handle 0x5627fd0a4348; line 1372 (connection #-5000)
still_running = 2
== Info (0): Could not resolve host: non-existing-host.haxx.se
== Info (1): STATE: CONNECT_PEND => CONNECT handle 0x5627fd0a4348; line 2999 (connection #-5000)
== Info (0): Curl_disconnect when inuse: 1
== Info (0): Expire cleared (transfer 0x5627fd09ea98)
== Info (1): Found bundle for host non-existing-host.haxx.se: 0x5627fd0753f8 [serially]
== Info (1): Connection #0 is still name resolving, can't reuse
== Info (1): No connections available in cache
== Info (1): No connections available.
== Info (1): STATE: CONNECT => CONNECT_PEND handle 0x5627fd0a4348; line 1372 (connection #-5000)
still_running = 1

Good output from older curl (and the program exits):

== Info (0): STATE: INIT => CONNECT handle 0x55cb42ff4af8; line 1361 (connection #-5000)
== Info (0): Added connection 0. The cache now contains 1 members
== Info (0): STATE: CONNECT => WAITRESOLVE handle 0x55cb42ff4af8; line 1402 (connection #0)
== Info (1): STATE: INIT => CONNECT handle 0x55cb42ffa3a8; line 1361 (connection #-5000)
== Info (1): Found bundle for host non-existing-host.haxx.se: 0x55cb42fcb3f8 [serially]
== Info (1): Server doesn't support multi-use (yet)
== Info (1): No connections available in cache
== Info (1): No connections available.
== Info (1): STATE: CONNECT => CONNECT_PEND handle 0x55cb42ffa3a8; line 1385 (connection #-5000)
still_running = 2
== Info (0): Could not resolve host: non-existing-host.haxx.se
== Info (1): STATE: CONNECT_PEND => CONNECT handle 0x55cb42ffa3a8; line 3111 (connection #-5000)
== Info (0): Closing connection 0
== Info (0): The cache now contains 0 members
== Info (0): Expire cleared (transfer 0x55cb42ff4af8)
== Info (1): Added connection 1. The cache now contains 1 members
== Info (1): STATE: CONNECT => WAITRESOLVE handle 0x55cb42ffa3a8; line 1402 (connection #1)
still_running = 1
== Info (1): Could not resolve host: non-existing-host.haxx.se
== Info (1): Closing connection 1
== Info (1): The cache now contains 0 members
== Info (1): Expire cleared (transfer 0x55cb42ffa3a8)
still_running = 0
@ngg ngg mentioned this issue Jul 23, 2019
Closed
@andreibica
Copy link

I'm having the same issue. I've reproduce it using a modified multi-uv.c example: just set the CURLMOPT_MAX_TOTAL_CONNECTIONS to 1 and run the example with a set of invalid domains.

@ngg
Copy link
Contributor Author

ngg commented Jul 25, 2019

So the problem is that multi_runsingle detects a stream_error and calls Curl_disconnect and then detach_connection. The former does not do anything since CONN_INUSE(conn) is true because conn->easyq still contains data (which will be only later removed in detach_connection). If I switch the order of these two calls, it seems to work, though I have no idea if it would cause any other trouble, or if there are other places where something like this should be called before Curl_disconnect.

ngg added a commit to tresorit/curl that referenced this issue Jul 25, 2019
@ngg
multi: call detach_connection before Curl_disconnect
e91dbe1 
Curl_disconnect bails out if conn->easyq is not empty,
detach_connection needs to be called first to remove
the current easy from the queue.

Fixes curl#4144
@ngg ngg mentioned this issue Jul 25, 2019
Closed
@bagder bagder self-assigned this Jul 25, 2019
@bagder bagder added the name lookup DNS and related tech label Jul 25, 2019
@bagder bagder closed this as completed in a55edce Jul 25, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bagder bagder

Labels
name lookup DNS and related tech
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

multi: call detach_connection before Curl_disconnect tresorit/curl
3 participants
@bagder @ngg @andreibica