From a21895d1452da32194a5fa54f55d0a3951c1c7ec Mon Sep 17 00:00:00 2001 From: Willem Toorop <willem@nlnetlabs.nl> Date: Tue, 7 Oct 2014 15:52:41 +0200 Subject: [PATCH] Fix timeout handling & simultaneous 4 & 6 requests Simultaneous 4 & 6 requests for async only. Also get rid of the postponing of callbacks hack. --- src/context.c | 56 +++++++++++++----------- src/general.c | 97 +++++++++++++----------------------------- src/request-internal.c | 26 ++++------- src/types-internal.h | 2 - 4 files changed, 68 insertions(+), 113 deletions(-) diff --git a/src/context.c b/src/context.c index 62deb4a9..1498b742 100755 --- a/src/context.c +++ b/src/context.c @@ -530,6 +530,15 @@ void getdns_handle_timeouts(struct getdns_event_base* base, /** call select and callbacks for that */ int getdns_handle_select(struct getdns_event_base* base, struct timeval* wait); +int +getdns_mini_event_settime(getdns_mini_event_extension *e) +{ + if (gettimeofday(e->base->time_tv, NULL) < 0) + return -1; + *e->base->time_secs = (time_t)e->base->time_tv->tv_sec; + return 0; +} + static void getdns_mini_event_timeout_cb(int fd, short bits, void *arg) { @@ -552,6 +561,8 @@ getdns_mini_event_schedule_timeout(getdns_context *context, void *ext, timeout_data->extension_timer = ev; getdns_event_set(ev, -1, EV_TIMEOUT, getdns_mini_event_timeout_cb, timeout_data); + + (void) getdns_mini_event_settime(e); (void) getdns_event_base_set(e->base, ev); (void) getdns_event_add(ev, &tv); return GETDNS_RETURN_GOOD; @@ -1537,19 +1548,16 @@ getdns_context_set_memory_functions(struct getdns_context *context, static void cancel_dns_req(getdns_dns_req * req) { - getdns_network_req *netreq = req->first_req; - while (netreq) { - if (netreq->state == NET_REQ_IN_FLIGHT) { - /* for ev based ub, this should always prevent - * the callback from firing */ - ub_cancel(req->context->unbound_ctx, netreq->unbound_id); - netreq->state = NET_REQ_CANCELED; - } else if (netreq->state == NET_REQ_NOT_SENT) { - netreq->state = NET_REQ_CANCELED; - } - netreq = netreq->next; - } - req->canceled = 1; + getdns_network_req *netreq = req->first_req; + while (netreq) { + if (netreq->unbound_id != -1) { + ub_cancel(req->context->unbound_ctx, + netreq->unbound_id); + netreq->unbound_id = -1; + } + netreq = netreq->next; + } + req->canceled = 1; } getdns_return_t @@ -1919,22 +1927,18 @@ uint32_t getdns_context_get_num_pending_requests(struct getdns_context* context, struct timeval* next_timeout) { + static struct timeval dummy = { 0, 0 }; RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); if (context->outbound_requests->count && - context->extension == (void *)&context->mini_event_extension.ext) { - - struct getdns_event_base *base = - context->mini_event_extension.base; - - if (gettimeofday(base->time_tv, NULL) >= 0) { - struct timeval dummy; - - *base->time_secs = (time_t) base->time_tv->tv_sec; - getdns_handle_timeouts(base, base->time_tv, - next_timeout ? next_timeout : &dummy); - } - } + context->extension == (void *)&context->mini_event_extension.ext && + getdns_mini_event_settime(&context->mini_event_extension) == 0) + + getdns_handle_timeouts( + context->mini_event_extension.base, + context->mini_event_extension.base->time_tv, + next_timeout ? next_timeout : &dummy); + return context->outbound_requests->count; } diff --git a/src/general.c b/src/general.c index 4aa88099..8869221e 100644 --- a/src/general.c +++ b/src/general.c @@ -51,7 +51,6 @@ /* declarations */ static void ub_resolve_callback(void* mydata, int err, struct ub_result* result); static getdns_return_t ub_resolve_timeout(void *arg); -static getdns_return_t ub_local_resolve_timeout(void *arg); static void handle_network_request_error(getdns_network_req * netreq, int err); static void handle_dns_request_complete(getdns_dns_req * dns_req); @@ -72,27 +71,6 @@ ub_resolve_timeout(void *arg) return getdns_context_request_timed_out(dns_req); } -static getdns_return_t -ub_local_resolve_timeout(void *arg) -{ - netreq_cb_data *cb_data = (netreq_cb_data *) arg; - - /* cleanup the local timer here since the memory may be - * invalid after calling ub_resolve_callback - */ - getdns_dns_req *dnsreq = cb_data->netreq->owner; - /* clear the timeout */ - - getdns_context_clear_timeout(dnsreq->context, &dnsreq->local_timeout); - - /* just call ub_resolve_callback */ - ub_resolve_callback(cb_data->netreq, cb_data->err, cb_data->ub_res); - - /* cleanup the state */ - GETDNS_FREE(dnsreq->my_mf, cb_data); - return GETDNS_RETURN_GOOD; -} - void priv_getdns_call_user_callback(getdns_dns_req *dns_req, struct getdns_dict *response) { @@ -139,7 +117,6 @@ submit_network_request(getdns_network_req * netreq) netreq, ub_resolve_callback, &(netreq->unbound_id)); - netreq->state = NET_REQ_IN_FLIGHT; return r; } @@ -148,49 +125,30 @@ ub_resolve_callback(void* arg, int err, struct ub_result* ub_res) // ub_resolve_callback(void *arg, int err, ldns_buffer * result, int sec, // char *bogus) { - getdns_network_req *netreq = (getdns_network_req *) arg; - if (err != 0) { - handle_network_request_error(netreq, err); - return; - } - /* if netreq->state == NET_REQ_NOT_SENT here, that implies - * that ub called us back immediately - probably from a local file. - * This most likely means that getdns_general has not returned - */ - if (netreq->state == NET_REQ_NOT_SENT) { - /* just do a very short timer since this was called immediately. - * we can make this less hacky, but it gets interesting when multiple - * netreqs need to be issued and some resolve immediately vs. not. - */ - getdns_dns_req *dnsreq = netreq->owner; - netreq_cb_data *cb_data = GETDNS_MALLOC(dnsreq->my_mf, netreq_cb_data); - cb_data->netreq = netreq; - cb_data->err = err; - cb_data->ub_res = ub_res; + getdns_network_req *netreq = (getdns_network_req *) arg; + getdns_dns_req *dnsreq = netreq->owner; - getdns_context_schedule_timeout(dnsreq->context, 1, - ub_local_resolve_timeout, cb_data, &dnsreq->local_timeout); + netreq->state = NET_REQ_FINISHED; + if (err != 0) { + handle_network_request_error(netreq, err); return; } - netreq->state = NET_REQ_FINISHED; /* parse */ - /* TODO: optimize */ - getdns_return_t r = getdns_apply_network_result(netreq, ub_res); - ub_resolve_free(ub_res); - if (r != GETDNS_RETURN_GOOD) { - handle_network_request_error(netreq, err); - } else { - /* is this the last request */ - if (!netreq->next) { - /* finished */ - handle_dns_request_complete(netreq->owner); - } else { - /* not finished - update to next request and ship it */ - getdns_dns_req *dns_req = netreq->owner; - dns_req->current_req = netreq->next; - submit_network_request(netreq->next); - } + if (getdns_apply_network_result(netreq, ub_res)) { + ub_resolve_free(ub_res); + handle_network_request_error(netreq, err); + return; } + ub_resolve_free(ub_res); + + netreq = dnsreq->first_req; + while (netreq) { + if (netreq->state != NET_REQ_FINISHED && + netreq->state != NET_REQ_CANCELED) + return; + netreq = netreq->next; + } + handle_dns_request_complete(dnsreq); } /* ub_resolve_callback */ getdns_return_t @@ -204,7 +162,8 @@ getdns_general_ub(struct getdns_context *context, int usenamespaces) { getdns_return_t gr; - int r; + int r = GETDNS_RETURN_GOOD; + getdns_network_req *netreq; if (!name) { return GETDNS_RETURN_INVALID_PARAMETER; @@ -236,14 +195,16 @@ getdns_general_ub(struct getdns_context *context, /* assign a timeout */ // req->ev_base = ev_base; // req->timeout = evtimer_new(ev_base, ub_resolve_timeout, req); - /* schedule the timeout */ - getdns_context_schedule_timeout(context, context->timeout, - ub_resolve_timeout, req, &req->timeout); + /* schedule the timeout */ + getdns_context_schedule_timeout(context, context->timeout, + ub_resolve_timeout, req, &req->timeout); /* issue the first network req */ - - r = submit_network_request(req->first_req); - + netreq = req->first_req; + while (r == GETDNS_RETURN_GOOD && netreq) { + r = submit_network_request(netreq); + netreq = netreq->next; + } if (r != 0) { /* clean up the request */ getdns_context_clear_outbound_request(req); diff --git a/src/request-internal.c b/src/request-internal.c index fb7f88f4..03ce8680 100644 --- a/src/request-internal.c +++ b/src/request-internal.c @@ -36,6 +36,7 @@ #include "config.h" #include "types-internal.h" #include "util-internal.h" +#include "gldns/rrdef.h" void network_req_free(getdns_network_req * net_req) @@ -94,7 +95,6 @@ dns_req_free(getdns_dns_req * req) net_req = next; } - getdns_context_clear_timeout(context, &req->local_timeout); getdns_context_clear_timeout(context, &req->timeout); /* free strduped name */ @@ -110,15 +110,13 @@ dns_req_new(struct getdns_context *context, getdns_dns_req *result = NULL; getdns_network_req *req = NULL; - - getdns_return_t r; - uint32_t klass; + uint32_t klass = GLDNS_RR_CLASS_IN; result = GETDNS_MALLOC(context->mf, getdns_dns_req); if (result == NULL) { return NULL; } - result->my_mf = context->mf; + result->my_mf = context->mf; result->name = getdns_strdup(&(result->my_mf), name); result->context = context; result->canceled = 0; @@ -127,19 +125,15 @@ dns_req_new(struct getdns_context *context, result->trans_id = ldns_get_random(); getdns_dict_copy(extensions, &result->extensions); - result->return_dnssec_status = context->return_dnssec_status; + result->return_dnssec_status = context->return_dnssec_status; /* will be set by caller */ result->user_pointer = NULL; result->user_callback = NULL; memset(&result->timeout, 0, sizeof(getdns_timeout_data_t)); - memset(&result->local_timeout, 0, sizeof(getdns_timeout_data_t)); /* check the specify_class extension */ - if ((r = getdns_dict_get_int(extensions, "specify_class", &klass)) - != GETDNS_RETURN_GOOD) { - klass = LDNS_RR_CLASS_IN; - } + (void) getdns_dict_get_int(extensions, "specify_class", &klass); /* create the requests */ req = network_req_new(result, request_type, klass, extensions); @@ -156,13 +150,11 @@ dns_req_new(struct getdns_context *context, (request_type == GETDNS_RRTYPE_A || request_type == GETDNS_RRTYPE_AAAA)) { - uint16_t next_req_type = - (request_type == - GETDNS_RRTYPE_A) ? GETDNS_RRTYPE_AAAA : GETDNS_RRTYPE_A; + uint16_t next_req_type = (request_type == GETDNS_RRTYPE_A) ? + GETDNS_RRTYPE_AAAA : GETDNS_RRTYPE_A; getdns_network_req *next_req = network_req_new(result, - next_req_type, - LDNS_RR_CLASS_IN, - extensions); + next_req_type, LDNS_RR_CLASS_IN, extensions); + if (!next_req) { dns_req_free(result); return NULL; diff --git a/src/types-internal.h b/src/types-internal.h index 3c97178a..497a488b 100644 --- a/src/types-internal.h +++ b/src/types-internal.h @@ -199,8 +199,6 @@ typedef struct getdns_dns_req getdns_transaction_t trans_id; getdns_timeout_data_t timeout; - /* Hack to prevent immediate callbacks */ - getdns_timeout_data_t local_timeout; /* dnssec status */ int return_dnssec_status;