- 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

diff --git a/daemon/worker.c b/daemon/worker.c
index 832278fc3d5923768584fefa7884695a4fc50328..eeb323c8426afe7fc440fc93e026601ffce8b38c 100644 (file)
--- 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 993c547b6f861d223d78ca11da0b32e185a536be..1eed0fc7a8bd9b0cd463d20c723b317b80e80120 100644 (file)
--- 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 b23d560ab28a5bcf8bffff43e6e09e1b99a97f67..e3da24dd8ed6d13cf4873ed6bd393b32490516d6 100644 (file)
--- 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 ba40d474dd222376ceb1b72e6b5fcc744090c14a..f34516d94f4c5302de80ddf8a2672bf132510441 100644 (file)
--- 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 ab18d2406e6a28967f497ba60848b7d0ea12bc36..316c4cf9c146e1c62b7b512c5d00c6153ca24b72 100644 (file)
--- 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 26dfaa8b068bda638eb255ba0d2bb8024b101408..180ff30697e2a08cd4e52b4cd35d7a4fe0a5d11e 100644 (file)
--- 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);