Skip to content

Commit

Permalink
urldata: move async resolver state from easy handle to connectdata
Browse files Browse the repository at this point in the history 
- resolving is done for a connection, not for every transfer
- save create/dup/free of a cares channel for each transfer
- check values of setopt calls against a local channel if no
  connection has been attached yet, when needed.

Closes #12198
  • Loading branch information
@icing @bagder
icing authored and bagder committed Oct 26, 2023
1 parent 910f740 commit 56a4db2
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 127 deletions.
119 changes: 73 additions & 46 deletions lib/asyn-ares.c
View file
Expand Up @@ -228,9 +228,9 @@ static void destroy_async_data(struct Curl_async *async);
void Curl_resolver_cancel(struct Curl_easy *data)
{
DEBUGASSERT(data);
if(data->state.async.resolver)
ares_cancel((ares_channel)data->state.async.resolver);
destroy_async_data(&data->state.async);
if(data->conn->resolve_async.resolver)
ares_cancel((ares_channel)data->conn->resolve_async.resolver);
destroy_async_data(&data->conn->resolve_async);
}

/*
Expand Down Expand Up @@ -278,14 +278,14 @@ int Curl_resolver_getsock(struct Curl_easy *data,
struct timeval timebuf;
struct timeval *timeout;
long milli;
int max = ares_getsock((ares_channel)data->state.async.resolver,
int max = ares_getsock((ares_channel)data->conn->resolve_async.resolver,
(ares_socket_t *)socks, MAX_SOCKSPEREASYHANDLE);

maxtime.tv_sec = CURL_TIMEOUT_RESOLVE;
maxtime.tv_usec = 0;

timeout = ares_timeout((ares_channel)data->state.async.resolver, &maxtime,
&timebuf);
timeout = ares_timeout((ares_channel)data->conn->resolve_async.resolver,
&maxtime, &timebuf);
milli = (long)curlx_tvtoms(timeout);
if(milli == 0)
milli += 10;
Expand Down Expand Up @@ -313,8 +313,8 @@ static int waitperform(struct Curl_easy *data, timediff_t timeout_ms)
int i;
int num = 0;

bitmask = ares_getsock((ares_channel)data->state.async.resolver, socks,
ARES_GETSOCK_MAXNUM);
bitmask = ares_getsock((ares_channel)data->conn->resolve_async.resolver,
socks, ARES_GETSOCK_MAXNUM);

for(i = 0; i < ARES_GETSOCK_MAXNUM; i++) {
pfd[i].events = 0;
Expand Down Expand Up @@ -344,12 +344,12 @@ static int waitperform(struct Curl_easy *data, timediff_t timeout_ms)
if(!nfds)
/* Call ares_process() unconditionally here, even if we simply timed out
above, as otherwise the ares name resolve won't timeout! */
ares_process_fd((ares_channel)data->state.async.resolver, ARES_SOCKET_BAD,
ARES_SOCKET_BAD);
ares_process_fd((ares_channel)data->conn->resolve_async.resolver,
ARES_SOCKET_BAD, ARES_SOCKET_BAD);
else {
/* move through the descriptors and ask for processing on them */
for(i = 0; i < num; i++)
ares_process_fd((ares_channel)data->state.async.resolver,
ares_process_fd((ares_channel)data->conn->resolve_async.resolver,
(pfd[i].revents & (POLLRDNORM|POLLIN))?
pfd[i].fd:ARES_SOCKET_BAD,
(pfd[i].revents & (POLLWRNORM|POLLOUT))?
Expand All @@ -368,7 +368,7 @@ static int waitperform(struct Curl_easy *data, timediff_t timeout_ms)
CURLcode Curl_resolver_is_resolved(struct Curl_easy *data,
struct Curl_dns_entry **dns)
{
struct thread_data *res = data->state.async.tdata;
struct thread_data *res = data->conn->resolve_async.tdata;
CURLcode result = CURLE_OK;

DEBUGASSERT(dns);
Expand Down Expand Up @@ -397,7 +397,7 @@ CURLcode Curl_resolver_is_resolved(struct Curl_easy *data,
ARES_ECANCELLED synchronously for all pending responses. This will
leave us with res->num_pending == 0, which is perfect for the next
block. */
ares_cancel((ares_channel)data->state.async.resolver);
ares_cancel((ares_channel)data->conn->resolve_async.resolver);
DEBUGASSERT(res->num_pending == 0);
}
#endif
Expand All @@ -408,12 +408,12 @@ CURLcode Curl_resolver_is_resolved(struct Curl_easy *data,
them */
res->temp_ai = NULL;

if(!data->state.async.dns)
if(!data->conn->resolve_async.dns)
result = Curl_resolver_error(data);
else
*dns = data->state.async.dns;
*dns = data->conn->resolve_async.dns;

destroy_async_data(&data->state.async);
destroy_async_data(&data->conn->resolve_async);
}

return result;
Expand Down Expand Up @@ -464,7 +464,8 @@ CURLcode Curl_resolver_wait_resolv(struct Curl_easy *data,
store.tv_sec = itimeout/1000;
store.tv_usec = (itimeout%1000)*1000;

tvp = ares_timeout((ares_channel)data->state.async.resolver, &store, &tv);
tvp = ares_timeout((ares_channel)data->conn->resolve_async.resolver,
&store, &tv);

/* use the timeout period ares returned to us above if less than one
second is left, otherwise just use 1000ms to make sure the progress
Expand All @@ -478,7 +479,7 @@ CURLcode Curl_resolver_wait_resolv(struct Curl_easy *data,
return CURLE_UNRECOVERABLE_POLL;
result = Curl_resolver_is_resolved(data, entry);

if(result || data->state.async.done)
if(result || data->conn->resolve_async.done)
break;

if(Curl_pgrsUpdate(data))
Expand All @@ -499,12 +500,12 @@ CURLcode Curl_resolver_wait_resolv(struct Curl_easy *data,
}
if(result)
/* failure, so we cancel the ares operation */
ares_cancel((ares_channel)data->state.async.resolver);
ares_cancel((ares_channel)data->conn->resolve_async.resolver);

/* Operation complete, if the lookup was successful we now have the entry
in the cache. */
if(entry)
*entry = data->state.async.dns;
*entry = data->conn->resolve_async.dns;

if(result)
/* close the connection, since we can't return failure here without
Expand Down Expand Up @@ -571,12 +572,13 @@ static void query_completed_cb(void *arg, /* (struct connectdata *) */
be valid so only defer it when we know the 'status' says its fine! */
return;

res = data->state.async.tdata;
res = data->conn->resolve_async.tdata;
if(res) {
res->num_pending--;

if(CURL_ASYNC_SUCCESS == status) {
struct Curl_addrinfo *ai = Curl_he2ai(hostent, data->state.async.port);
struct Curl_addrinfo *ai = Curl_he2ai(hostent,
data->conn->resolve_async.port);
if(ai) {
compound_results(res, ai);
}
Expand Down Expand Up @@ -727,7 +729,7 @@ static void addrinfo_cb(void *arg, int status, int timeouts,
struct ares_addrinfo *result)
{
struct Curl_easy *data = (struct Curl_easy *)arg;
struct thread_data *res = data->state.async.tdata;
struct thread_data *res = data->conn->resolve_async.tdata;
(void)timeouts;
if(ARES_SUCCESS == status) {
res->temp_ai = ares2addr(result->nodes);
Expand Down Expand Up @@ -758,12 +760,12 @@ struct Curl_addrinfo *Curl_resolver_getaddrinfo(struct Curl_easy *data,
res = calloc(sizeof(struct thread_data) + namelen, 1);
if(res) {
strcpy(res->hostname, hostname);
data->state.async.hostname = res->hostname;
data->state.async.port = port;
data->state.async.done = FALSE; /* not done */
data->state.async.status = 0; /* clear */
data->state.async.dns = NULL; /* clear */
data->state.async.tdata = res;
data->conn->resolve_async.hostname = res->hostname;
data->conn->resolve_async.port = port;
data->conn->resolve_async.done = FALSE; /* not done */
data->conn->resolve_async.status = 0; /* clear */
data->conn->resolve_async.dns = NULL; /* clear */
data->conn->resolve_async.tdata = res;

/* initial status - failed */
res->last_status = ARES_ENOTFOUND;
Expand Down Expand Up @@ -793,8 +795,8 @@ struct Curl_addrinfo *Curl_resolver_getaddrinfo(struct Curl_easy *data,
hints.ai_flags = ARES_AI_NUMERICSERV;
msnprintf(service, sizeof(service), "%d", port);
res->num_pending = 1;
ares_getaddrinfo((ares_channel)data->state.async.resolver, hostname,
service, &hints, addrinfo_cb, data);
ares_getaddrinfo((ares_channel)data->conn->resolve_async.resolver,
hostname, service, &hints, addrinfo_cb, data);
}
#else

Expand All @@ -804,18 +806,18 @@ struct Curl_addrinfo *Curl_resolver_getaddrinfo(struct Curl_easy *data,
res->num_pending = 2;

/* areschannel is already setup in the Curl_open() function */
ares_gethostbyname((ares_channel)data->state.async.resolver, hostname,
PF_INET, query_completed_cb, data);
ares_gethostbyname((ares_channel)data->state.async.resolver, hostname,
PF_INET6, query_completed_cb, data);
ares_gethostbyname((ares_channel)data->conn->resolve_async.resolver,
hostname, PF_INET, query_completed_cb, data);
ares_gethostbyname((ares_channel)data->conn->resolve_async.resolver,
hostname, PF_INET6, query_completed_cb, data);
}
else
#endif
{
res->num_pending = 1;

/* areschannel is already setup in the Curl_open() function */
ares_gethostbyname((ares_channel)data->state.async.resolver,
ares_gethostbyname((ares_channel)data->conn->resolve_async.resolver,
hostname, PF_INET,
query_completed_cb, data);
}
Expand All @@ -829,6 +831,7 @@ CURLcode Curl_set_dns_servers(struct Curl_easy *data,
char *servers)
{
CURLcode result = CURLE_NOT_BUILT_IN;
ares_channel channel, lchannel = NULL;
int ares_result;

/* If server is NULL or empty, this would purge all DNS servers
Expand All @@ -841,11 +844,23 @@ CURLcode Curl_set_dns_servers(struct Curl_easy *data,
return CURLE_OK;

#ifdef HAVE_CARES_SERVERS_CSV
if(data->conn)
channel = data->conn->resolve_async.resolver;
else {
/* we are called by setopt on a data without a connection (yet). In that
* case we set the value on a local instance for checking.
* The configured data options are set when the connection for this
* transfer is created. */
result = Curl_resolver_init(data, (void **)&lchannel);
if(result)
goto out;
channel = lchannel;
}

#ifdef HAVE_CARES_PORTS_CSV
ares_result = ares_set_servers_ports_csv(data->state.async.resolver,
servers);
ares_result = ares_set_servers_ports_csv(channel, servers);
#else
ares_result = ares_set_servers_csv(data->state.async.resolver, servers);
ares_result = ares_set_servers_csv(channel, servers);
#endif
switch(ares_result) {
case ARES_SUCCESS:
Expand All @@ -861,6 +876,9 @@ CURLcode Curl_set_dns_servers(struct Curl_easy *data,
result = CURLE_BAD_FUNCTION_ARGUMENT;
break;
}
out:
if(lchannel)
Curl_resolver_cleanup(lchannel);
#else /* too old c-ares version! */
(void)data;
(void)(ares_result);
Expand All @@ -872,11 +890,14 @@ CURLcode Curl_set_dns_interface(struct Curl_easy *data,
const char *interf)
{
#ifdef HAVE_CARES_LOCAL_DEV
if(!interf)
interf = "";

ares_set_local_dev((ares_channel)data->state.async.resolver, interf);
if(data->conn) {
/* not a setopt test run, set the value */
if(!interf)
interf = "";

ares_set_local_dev((ares_channel)data->conn->resolve_async.resolver,
interf);
}
return CURLE_OK;
#else /* c-ares version too old! */
(void)data;
Expand All @@ -900,8 +921,11 @@ CURLcode Curl_set_dns_local_ip4(struct Curl_easy *data,
}
}

ares_set_local_ip4((ares_channel)data->state.async.resolver,
ntohl(a4.s_addr));
if(data->conn) {
/* not a setopt test run, set the value */
ares_set_local_ip4((ares_channel)data->conn->resolve_async.resolver,
ntohl(a4.s_addr));
}

return CURLE_OK;
#else /* c-ares version too old! */
Expand All @@ -927,7 +951,10 @@ CURLcode Curl_set_dns_local_ip6(struct Curl_easy *data,
}
}

ares_set_local_ip6((ares_channel)data->state.async.resolver, a6);
if(data->conn) {
/* not a setopt test run, set the value */
ares_set_local_ip6((ares_channel)data->conn->resolve_async.resolver, a6);
}

return CURLE_OK;
#else /* c-ares version too old! */
Expand Down

2 comments on commit 56a4db2

@gvanem
Copy link
Contributor

@gvanem gvanem commented on 56a4db2 Oct 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this PR or something older broke building with -DUSE_SYNC_DNS
(and no -DUSE_THREADS_WIN32 -DUSE_CARES -DUSE_ARES), But compiling hostip6.c, gives this compile error:

hostip6.c(152,17): error: use of undeclared identifier 'conn'
  152 |   dump_addrinfo(conn, res);
      |                 ^

Should be:

--- hostip6.c 2023-01-05 13:01:11
+++ hostip6.c 2023-10-27 09:30:04
@@ -149,7 +149,7 @@
     Curl_addrinfo_set_port(res, port);
   }

-  dump_addrinfo(conn, res);
+  dump_addrinfo(data->conn, res);

   return res;
 }

AFAICS.

@jay
Copy link
Member

@jay jay commented on 56a4db2 Oct 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compiling hostip6.c, gives this compile error:

Thanks see #12212

Please sign in to comment.