Fixed ticket #686: Failure to cancel outstanding DNS SRV query job (API change)

git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@2387 74dad513-b988-da41-8d7b-12977e46ad98

4 files changed, 105 insertions, 40 deletions

diff --git a/pjlib-util/include/pjlib-util/srv_resolver.h b/pjlib-util/include/pjlib-util/srv_resolver.h
index d1c896eb..1ff8f903 100644
--- a/pjlib-util/include/pjlib-util/srv_resolver.h
+++ b/pjlib-util/include/pjlib-util/srv_resolver.h
@@ -137,6 +137,9 @@ typedef struct pj_dns_srv_record
 } pj_dns_srv_record;
 
 
+/** Opaque declaration for DNS SRV query */
+typedef struct pj_dns_srv_async_query pj_dns_srv_async_query;
+
 /**
  * Type of callback function to receive notification from the resolver
  * when the resolution process completes.
@@ -184,7 +187,20 @@ PJ_DECL(pj_status_t) pj_dns_srv_resolve(const pj_str_t *domain_name,
 					unsigned option,
 					void *token,
 					pj_dns_srv_resolver_cb *cb,
-					pj_dns_async_query **p_query);
+					pj_dns_srv_async_query **p_query);
+
+
+/**
+ * Cancel an outstanding DNS SRV query.
+ *
+ * @param query	    The pending asynchronous query to be cancelled.
+ * @param notify    If non-zero, the callback will be called with failure
+ *		    status to notify that the query has been cancelled.
+ *
+ * @return	    PJ_SUCCESS on success, or the appropriate error code,
+ */
+PJ_DECL(pj_status_t) pj_dns_srv_cancel_query(pj_dns_srv_async_query *query,
+					     pj_bool_t notify);
 
 
 /**
diff --git a/pjlib-util/src/pjlib-util/srv_resolver.c b/pjlib-util/src/pjlib-util/srv_resolver.c
index 368e52b2..71077a46 100644
--- a/pjlib-util/src/pjlib-util/srv_resolver.c
+++ b/pjlib-util/src/pjlib-util/srv_resolver.c
@@ -21,6 +21,7 @@
 #include <pj/array.h>
 #include <pj/assert.h>
 #include <pj/log.h>
+#include <pj/os.h>
 #include <pj/pool.h>
 #include <pj/rand.h>
 #include <pj/string.h>
@@ -30,9 +31,17 @@
 
 #define ADDR_MAX_COUNT	    PJ_DNS_MAX_IP_IN_A_REC
 
+struct common
+{
+    pj_dns_type		     type;	    /**< Type of this structure.*/
+};
+
 struct srv_target
 {
+    struct common	    common;
+    pj_dns_srv_async_query *parent;
     pj_str_t		    target_name;
+    pj_dns_async_query	   *q_a;
     char		    target_buf[PJ_MAX_HOSTNAME];
     pj_str_t		    cname;
     char		    cname_buf[PJ_MAX_HOSTNAME];
@@ -44,13 +53,15 @@ struct srv_target
     pj_in_addr		    addr[ADDR_MAX_COUNT];
 };
 
-typedef struct pj_dns_srv_resolver_job
+struct pj_dns_srv_async_query
 {
+    struct common	     common;
     char		    *objname;
 
-    pj_dns_resolver	    *resolver;	    /**< Resolver SIP instance.	    */
     pj_dns_type		     dns_state;	    /**< DNS type being resolved.   */
+    pj_dns_resolver	    *resolver;	    /**< Resolver SIP instance.	    */
     void		    *token;
+    pj_dns_async_query	    *q_srv;
     pj_dns_srv_resolver_cb  *cb;
     pj_status_t		     last_error;
 
@@ -67,7 +78,7 @@ typedef struct pj_dns_srv_resolver_job
     /* Number of hosts in SRV records that the IP address has been resolved */
     unsigned		     host_resolved;
 
-} pj_dns_srv_resolver_job;
+};
 
 
 /* Async resolver callback, forward decl. */
@@ -88,11 +99,11 @@ PJ_DEF(pj_status_t) pj_dns_srv_resolve( const pj_str_t *domain_name,
 					unsigned option,
 					void *token,
 					pj_dns_srv_resolver_cb *cb,
-					pj_dns_async_query **p_query)
+					pj_dns_srv_async_query **p_query)
 {
     int len;
     pj_str_t target_name;
-    pj_dns_srv_resolver_job *query_job;
+    pj_dns_srv_async_query *query_job;
     pj_status_t status;
 
     PJ_ASSERT_RETURN(domain_name && domain_name->slen &&
@@ -111,7 +122,8 @@ PJ_DEF(pj_status_t) pj_dns_srv_resolve( const pj_str_t *domain_name,
 
 
     /* Build the query_job state */
-    query_job = PJ_POOL_ZALLOC_T(pool, pj_dns_srv_resolver_job);
+    query_job = PJ_POOL_ZALLOC_T(pool, pj_dns_srv_async_query);
+    query_job->common.type = PJ_DNS_TYPE_SRV;
     query_job->objname = target_name.ptr;
     query_job->resolver = resolver;
     query_job->token = token;
@@ -135,11 +147,45 @@ PJ_DEF(pj_status_t) pj_dns_srv_resolve( const pj_str_t *domain_name,
     status = pj_dns_resolver_start_query(resolver, &target_name, 
 				         query_job->dns_state, 0, 
 					 &dns_callback,
-    					 query_job, p_query);
+    					 query_job, &query_job->q_srv);
+    if (status==PJ_SUCCESS && p_query)
+	*p_query = query_job;
+
     return status;
 }
 
 
+/*
+ * Cancel pending query.
+ */
+PJ_DEF(pj_status_t) pj_dns_srv_cancel_query(pj_dns_srv_async_query *query,
+					    pj_bool_t notify)
+{
+    pj_bool_t has_pending = PJ_FALSE;
+    unsigned i;
+
+    if (query->q_srv) {
+	pj_dns_resolver_cancel_query(query->q_srv, PJ_FALSE);
+	query->q_srv = NULL;
+	has_pending = PJ_TRUE;
+    }
+
+    for (i=0; i<query->srv_cnt; ++i) {
+	struct srv_target *srv = &query->srv[i];
+	if (srv->q_a) {
+	    pj_dns_resolver_cancel_query(srv->q_a, PJ_FALSE);
+	    srv->q_a = NULL;
+	    has_pending = PJ_TRUE;
+	}
+    }
+
+    if (has_pending && notify && query->cb) {
+	(*query->cb)(query->token, PJ_ECANCELLED, NULL);
+    }
+
+    return has_pending? PJ_SUCCESS : PJ_EINVALIDOP;
+}
+
 
 #define SWAP(type,ptr1,ptr2) if (ptr1 != ptr2) { \
 				type tmp; \
@@ -152,7 +198,7 @@ PJ_DEF(pj_status_t) pj_dns_srv_resolve( const pj_str_t *domain_name,
 
 
 /* Build server entries in the query_job based on received SRV response */
-static void build_server_entries(pj_dns_srv_resolver_job *query_job, 
+static void build_server_entries(pj_dns_srv_async_query *query_job, 
 				 pj_dns_parsed_packet *response)
 {
     unsigned i;
@@ -356,23 +402,28 @@ static void build_server_entries(pj_dns_srv_resolver_job *query_job,
 
 
 /* Start DNS A record queries for all SRV records in the query_job structure */
-static pj_status_t resolve_hostnames(pj_dns_srv_resolver_job *query_job)
+static pj_status_t resolve_hostnames(pj_dns_srv_async_query *query_job)
 {
     unsigned i;
     pj_status_t err=PJ_SUCCESS, status;
 
     query_job->dns_state = PJ_DNS_TYPE_A;
     for (i=0; i<query_job->srv_cnt; ++i) {
+	struct srv_target *srv = &query_job->srv[i];
+
 	PJ_LOG(5, (query_job->objname, 
 		   "Starting async DNS A query_job for %.*s",
-		   (int)query_job->srv[i].target_name.slen, 
-		   query_job->srv[i].target_name.ptr));
+		   (int)srv->target_name.slen, 
+		   srv->target_name.ptr));
+
+	srv->common.type = PJ_DNS_TYPE_A;
+	srv->parent = query_job;
 
 	status = pj_dns_resolver_start_query(query_job->resolver,
-					     &query_job->srv[i].target_name,
+					     &srv->target_name,
 					     PJ_DNS_TYPE_A, 0,
 					     &dns_callback,
-					     query_job, NULL);
+					     srv, &srv->q_a);
 	if (status != PJ_SUCCESS) {
 	    query_job->host_resolved++;
 	    err = status;
@@ -390,15 +441,30 @@ static void dns_callback(void *user_data,
 			 pj_status_t status,
 			 pj_dns_parsed_packet *pkt)
 {
-    pj_dns_srv_resolver_job *query_job = (pj_dns_srv_resolver_job*) user_data;
+    struct common *common = (struct common*) user_data;
+    pj_dns_srv_async_query *query_job;
+    struct srv_target *srv = NULL;
     unsigned i;
 
-    /* Proceed to next stage */
+    if (common->type == PJ_DNS_TYPE_SRV) {
+	query_job = (pj_dns_srv_async_query*) common;
+	srv = NULL;
+    } else if (common->type == PJ_DNS_TYPE_A) {
+	srv = (struct srv_target*) common;
+	query_job = srv->parent;
+    } else {
+	pj_assert(!"Unexpected user data!");
+	query_job->last_error = status = PJ_EINVALIDOP;
+	goto on_error;
+    }
 
+    /* Proceed to next stage */
     if (query_job->dns_state == PJ_DNS_TYPE_SRV) {
 
 	/* We are getting SRV response */
 
+	query_job->q_srv = NULL;
+
 	if (status == PJ_SUCCESS && pkt->hdr.anscount != 0) {
 	    /* Got SRV response, build server entry. If A records are available
 	     * in additional records section of the DNS response, save them too.
@@ -468,10 +534,11 @@ static void dns_callback(void *user_data,
 
     } else if (query_job->dns_state == PJ_DNS_TYPE_A) {
 
+	/* Clear the outstanding job */
+	srv->q_a = NULL;
+
 	/* Check that we really have answer */
 	if (status==PJ_SUCCESS && pkt->hdr.anscount != 0) {
-	    unsigned srv_idx;
-	    struct srv_target *srv = NULL;
 	    pj_dns_a_record rec;
 
 	    /* Parse response */
@@ -481,24 +548,6 @@ static void dns_callback(void *user_data,
 
 	    pj_assert(rec.addr_count != 0);
 
-	    /* Find which server entry to be updated. */
-	    for (srv_idx=0; srv_idx<query_job->srv_cnt; ++srv_idx) {
-		srv = &query_job->srv[srv_idx];
-		if (pj_stricmp(&rec.name, &srv->target_name)==0) {
-		    break;
-		}
-	    }
-
-	    if (srv_idx==query_job->srv_cnt) {
-		/* The DNS A response doesn't match any server names
-		 * we're querying!
-		 */
-		status = PJLIB_UTIL_EDNSINANSWER;
-		goto on_error;
-	    }
-
-	    srv = &query_job->srv[srv_idx];
-
 	    /* Update CNAME alias, if present. */
 	    if (rec.alias.slen) {
 		pj_assert(rec.alias.slen <= (int)sizeof(srv->cname_buf));
diff --git a/pjnath/src/pjnath/stun_sock.c b/pjnath/src/pjnath/stun_sock.c
index e27c20ed..d0703b93 100644
--- a/pjnath/src/pjnath/stun_sock.c
+++ b/pjnath/src/pjnath/stun_sock.c
@@ -47,7 +47,7 @@ struct pj_stun_sock
     pj_sockaddr		 srv_addr;	/* Resolved server addr	    */
     pj_sockaddr		 mapped_addr;	/* Our public address	    */
 
-    pj_dns_async_query	*q;		/* Pending DNS query	    */
+    pj_dns_srv_async_query *q;		/* Pending DNS query	    */
     pj_sock_t		 sock_fd;	/* Socket descriptor	    */
     pj_activesock_t	*active_sock;	/* Active socket object	    */
     pj_ioqueue_op_key_t	 send_key;	/* Default send key for app */
@@ -370,7 +370,7 @@ PJ_DEF(pj_status_t) pj_stun_sock_start( pj_stun_sock *stun_sock,
 PJ_DEF(pj_status_t) pj_stun_sock_destroy(pj_stun_sock *stun_sock)
 {
     if (stun_sock->q) {
-	pj_dns_resolver_cancel_query(stun_sock->q, PJ_FALSE);
+	pj_dns_srv_cancel_query(stun_sock->q, PJ_FALSE);
 	stun_sock->q = NULL;
     }
 
diff --git a/pjnath/src/pjnath/turn_session.c b/pjnath/src/pjnath/turn_session.c
index de6d4b24..7d45db41 100644
--- a/pjnath/src/pjnath/turn_session.c
+++ b/pjnath/src/pjnath/turn_session.c
@@ -86,7 +86,7 @@ struct pj_turn_session
     pj_timer_heap_t	*timer_heap;
     pj_timer_entry	 timer;
 
-    pj_dns_async_query	*dns_async;
+    pj_dns_srv_async_query *dns_async;
     pj_uint16_t		 default_port;
 
     pj_uint16_t		 af;
@@ -342,7 +342,7 @@ static void sess_shutdown(pj_turn_session *sess,
 	break;
     case PJ_TURN_STATE_RESOLVING:
 	if (sess->dns_async != NULL) {
-	    pj_dns_resolver_cancel_query(sess->dns_async, PJ_FALSE);
+	    pj_dns_srv_cancel_query(sess->dns_async, PJ_FALSE);
 	    sess->dns_async = NULL;
 	}
 	break;