From 9046a52364a48d275b9190440cdb01f8d7d82f43 Mon Sep 17 00:00:00 2001 From: Wouter Wijngaards Date: Tue, 31 Jul 2012 11:33:06 +0000 Subject: [PATCH] - Fix that enables modules to register twice for the same serviced_query, without race conditions or administration issues. This should not happen with the current codebase, but it is robust. git-svn-id: file:///svn/unbound/trunk@2730 be551aaa-1e26-0410-a405-d3ace91eadb9 --- daemon/worker.c | 13 +------------ doc/Changelog | 3 +++ libunbound/libworker.c | 13 +------------ services/outside_network.c | 33 +++++++++------------------------ services/outside_network.h | 5 +---- testcode/fake_event.c | 3 +-- 6 files changed, 16 insertions(+), 54 deletions(-) diff --git a/daemon/worker.c b/daemon/worker.c index 832278fc3..eeb323c84 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -1243,17 +1243,6 @@ worker_delete(struct worker* worker) free(worker); } -/** compare outbound entry qstates */ -static int -outbound_entry_compare(void* a, void* b) -{ - struct outbound_entry* e1 = (struct outbound_entry*)a; - struct outbound_entry* e2 = (struct outbound_entry*)b; - if(e1->qstate == e2->qstate) - return 1; - return 0; -} - struct outbound_entry* worker_send_query(uint8_t* qname, size_t qnamelen, uint16_t qtype, uint16_t qclass, uint16_t flags, int dnssec, int want_dnssec, @@ -1270,7 +1259,7 @@ worker_send_query(uint8_t* qname, size_t qnamelen, uint16_t qtype, qnamelen, qtype, qclass, flags, dnssec, want_dnssec, q->env->cfg->tcp_upstream, q->env->cfg->ssl_upstream, addr, addrlen, zone, zonelen, worker_handle_service_reply, e, - worker->back->udp_buff, &outbound_entry_compare); + worker->back->udp_buff); if(!e->qsent) { return NULL; } diff --git a/doc/Changelog b/doc/Changelog index 993c547b6..1eed0fc7a 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,5 +1,8 @@ 31 July 2012: Wouter - Improved forward-first and stub-first documentation. + - Fix that enables modules to register twice for the same + serviced_query, without race conditions or administration issues. + This should not happen with the current codebase, but it is robust. 30 July 2012: Wouter - tag 1.4.18rc2. diff --git a/libunbound/libworker.c b/libunbound/libworker.c index b23d560ab..e3da24dd8 100644 --- a/libunbound/libworker.c +++ b/libunbound/libworker.c @@ -705,17 +705,6 @@ void libworker_alloc_cleanup(void* arg) slabhash_clear(w->env->msg_cache); } -/** compare outbound entry qstates */ -static int -outbound_entry_compare(void* a, void* b) -{ - struct outbound_entry* e1 = (struct outbound_entry*)a; - struct outbound_entry* e2 = (struct outbound_entry*)b; - if(e1->qstate == e2->qstate) - return 1; - return 0; -} - struct outbound_entry* libworker_send_query(uint8_t* qname, size_t qnamelen, uint16_t qtype, uint16_t qclass, uint16_t flags, int dnssec, int want_dnssec, struct sockaddr_storage* addr, socklen_t addrlen, @@ -731,7 +720,7 @@ struct outbound_entry* libworker_send_query(uint8_t* qname, size_t qnamelen, qnamelen, qtype, qclass, flags, dnssec, want_dnssec, q->env->cfg->tcp_upstream, q->env->cfg->ssl_upstream, addr, addrlen, zone, zonelen, libworker_handle_service_reply, e, - w->back->udp_buff, &outbound_entry_compare); + w->back->udp_buff); if(!e->qsent) { return NULL; } diff --git a/services/outside_network.c b/services/outside_network.c index ba40d474d..f34516d94 100644 --- a/services/outside_network.c +++ b/services/outside_network.c @@ -1441,7 +1441,7 @@ static void serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c, struct comm_reply* rep) { - struct service_callback* p = sq->cblist, *n; + struct service_callback* p; int dobackup = (sq->cblist && sq->cblist->next); /* >1 cb*/ uint8_t *backup_p = NULL; size_t backlen = 0; @@ -1500,8 +1500,9 @@ serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c, } sq->outnet->svcd_overhead = backlen; } - while(p) { - n = p->next; + /* test the actual sq->cblist, because the next elem could be deleted*/ + while((p=sq->cblist) != NULL) { + sq->cblist = p->next; /* remove this element */ if(dobackup && c) { ldns_buffer_clear(c->buffer); ldns_buffer_write(c->buffer, backup_p, backlen); @@ -1509,7 +1510,7 @@ serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c, } fptr_ok(fptr_whitelist_serviced_query(p->cb)); (void)(*p->cb)(c, p->cb_arg, error, rep); - p = n; + free(p); } if(backup_p) { free(backup_p); @@ -1783,37 +1784,21 @@ serviced_udp_callback(struct comm_point* c, void* arg, int error, return 0; } -/** find callback in list */ -static struct service_callback* -callback_list_find(struct serviced_query* sq, void* cb_arg, - int (*arg_compare)(void*,void*)) -{ - struct service_callback* p; - for(p = sq->cblist; p; p = p->next) { - if(arg_compare(p->cb_arg, cb_arg)) - return p; - } - return NULL; -} - struct serviced_query* outnet_serviced_query(struct outside_network* outnet, uint8_t* qname, size_t qnamelen, uint16_t qtype, uint16_t qclass, uint16_t flags, int dnssec, int want_dnssec, int tcp_upstream, int ssl_upstream, struct sockaddr_storage* addr, socklen_t addrlen, uint8_t* zone, size_t zonelen, comm_point_callback_t* callback, - void* callback_arg, ldns_buffer* buff, int (*arg_compare)(void*,void*)) + void* callback_arg, ldns_buffer* buff) { struct serviced_query* sq; struct service_callback* cb; serviced_gen_query(buff, qname, qnamelen, qtype, qclass, flags); sq = lookup_serviced(outnet, buff, dnssec, addr, addrlen); - if(sq) { - /* see if it is a duplicate notification request for cb_arg */ - if(callback_list_find(sq, callback_arg, arg_compare)) { - return sq; - } - } + /* duplicate entries are inclded in the callback list, because + * there is a counterpart registration by our caller that needs to + * be doubly-removed (with callbacks perhaps). */ if(!(cb = (struct service_callback*)malloc(sizeof(*cb)))) return NULL; if(!sq) { diff --git a/services/outside_network.h b/services/outside_network.h index ab18d2406..316c4cf9c 100644 --- a/services/outside_network.h +++ b/services/outside_network.h @@ -468,8 +468,6 @@ void pending_delete(struct outside_network* outnet, struct pending* p); authoritative. * @param zonelen: length of zone. * @param buff: scratch buffer to create query contents in. Empty on exit. - * @param arg_compare: function to compare callback args, return true if - * identical. It is given the callback_arg and args that are listed. * @return 0 on error, or pointer to serviced query that is used to answer * this serviced query may be shared with other callbacks as well. */ @@ -478,8 +476,7 @@ struct serviced_query* outnet_serviced_query(struct outside_network* outnet, uint16_t flags, int dnssec, int want_dnssec, int tcp_upstream, int ssl_upstream, struct sockaddr_storage* addr, socklen_t addrlen, uint8_t* zone, size_t zonelen, comm_point_callback_t* callback, - void* callback_arg, ldns_buffer* buff, - int (*arg_compare)(void*,void*)); + void* callback_arg, ldns_buffer* buff); /** * Remove service query callback. diff --git a/testcode/fake_event.c b/testcode/fake_event.c index 26dfaa8b0..180ff3069 100644 --- a/testcode/fake_event.c +++ b/testcode/fake_event.c @@ -1041,14 +1041,13 @@ struct serviced_query* outnet_serviced_query(struct outside_network* outnet, int ATTR_UNUSED(tcp_upstream), int ATTR_UNUSED(ssl_upstream), struct sockaddr_storage* addr, socklen_t addrlen, uint8_t* zone, size_t zonelen, comm_point_callback_t* callback, void* callback_arg, - ldns_buffer* ATTR_UNUSED(buff), int (*arg_compare)(void*,void*)) + ldns_buffer* ATTR_UNUSED(buff)) { struct replay_runtime* runtime = (struct replay_runtime*)outnet->base; struct fake_pending* pend = (struct fake_pending*)calloc(1, sizeof(struct fake_pending)); char z[256]; ldns_status status; - (void)arg_compare; log_assert(pend); log_nametypeclass(VERB_OPS, "pending serviced query", qname, qtype, qclass); -- 2.47.3