-ensure clean DNS shutdown, fix DNS-STUN termination issues

3 files changed, 142 insertions, 77 deletions

diff --git a/src/nat/nat.c b/src/nat/nat.c
index 6e9fcfd0e..9a9ae18ac 100644
--- a/src/nat/nat.c
+++ b/src/nat/nat.c
@@ -429,7 +429,7 @@ struct GNUNET_NAT_Handle
   /**
    * STUN request task
    */
-  struct GNUNET_SCHEDULER_Task * stun_task;
+  struct GNUNET_SCHEDULER_Task *stun_task;
 
   /**
    * Head of List of STUN servers
@@ -460,15 +460,6 @@ start_gnunet_nat_server (struct GNUNET_NAT_Handle *h);
 
 
 /**
- * Call task to process STUN
- *
- * @param cls handle to NAT
- */
-static void
-process_stun (void *cls);
-
-
-/**
  * Remove all addresses from the list of 'local' addresses
  * that originated from the given source.
  *
@@ -688,7 +679,8 @@ process_external_ip (void *cls,
     /* Current iteration is over, remove 'old' IPs now */
     LOG (GNUNET_ERROR_TYPE_DEBUG,
          "Purging old IPs for external address\n");
-    remove_from_address_list_by_source (h, LAL_EXTERNAL_IP_OLD);
+    remove_from_address_list_by_source (h,
+                                        LAL_EXTERNAL_IP_OLD);
     if (1 == inet_pton (AF_INET,
                         h->external_address,
                         &dummy))
@@ -704,9 +696,13 @@ process_external_ip (void *cls,
   }
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Got IP `%s' for external address `%s'\n",
-       GNUNET_a2s (addr, addrlen),
+       GNUNET_a2s (addr,
+                   addrlen),
        h->external_address);
-  add_to_address_list (h, LAL_EXTERNAL_IP, addr, addrlen);
+  add_to_address_list (h,
+                       LAL_EXTERNAL_IP,
+                       addr,
+                       addrlen);
 }
 
 
@@ -740,10 +736,14 @@ process_hostname_ip (void *cls,
     h->hostname_dns = NULL;
     h->hostname_task =
         GNUNET_SCHEDULER_add_delayed (h->hostname_dns_frequency,
-                                      &resolve_hostname, h);
+                                      &resolve_hostname,
+                                      h);
     return;
   }
-  add_to_address_list (h, LAL_HOSTNAME_DNS, addr, addrlen);
+  add_to_address_list (h,
+                       LAL_HOSTNAME_DNS,
+                       addr,
+                       addrlen);
 }
 
 
@@ -1204,7 +1204,7 @@ static void
 process_stun (void *cls)
 {
   struct GNUNET_NAT_Handle *h = cls;
-  struct StunServerList* elem = h->actual_stun_server;
+  struct StunServerList *elem = h->actual_stun_server;
 
   h->stun_task = NULL;
   /* Make the request */
@@ -1260,9 +1260,12 @@ resolve_hostname (void *cls)
 
   h->hostname_task = NULL;
   remove_from_address_list_by_source (h, LAL_HOSTNAME_DNS);
+  GNUNET_assert (NULL == h->hostname_dns);
   h->hostname_dns =
-      GNUNET_RESOLVER_hostname_resolve (AF_UNSPEC, HOSTNAME_RESOLVE_TIMEOUT,
-                                        &process_hostname_ip, h);
+      GNUNET_RESOLVER_hostname_resolve (AF_UNSPEC,
+                                        HOSTNAME_RESOLVE_TIMEOUT,
+                                        &process_hostname_ip,
+                                        h);
 }
 
 
@@ -1285,10 +1288,13 @@ resolve_dns (void *cls)
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Resolving external address `%s'\n",
        h->external_address);
+  GNUNET_assert (NULL == h->ext_dns);
   h->ext_dns =
-      GNUNET_RESOLVER_ip_get (h->external_address, AF_INET,
+      GNUNET_RESOLVER_ip_get (h->external_address,
+                              AF_INET,
                               GNUNET_TIME_UNIT_MINUTES,
-                              &process_external_ip, h);
+                              &process_external_ip,
+                              h);
 }
 
 
@@ -1636,7 +1642,6 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg,
     size_t pos_port;
 
     h->socket = sock;
-    h->actual_stun_server = NULL;
     stun_servers = NULL;
     /* Lets process the servers*/
     (void) GNUNET_CONFIGURATION_get_value_string (cfg,
@@ -1644,9 +1649,6 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg,
                                                   "STUN_SERVERS",
                                                   &stun_servers);
     urls = 0;
-    h->stun_servers_head = NULL;
-    h->stun_servers_tail = NULL;
-    h->actual_stun_server = NULL;
     if ( (NULL != stun_servers) &&
          (strlen (stun_servers) > 0) )
     {
@@ -1665,8 +1667,8 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg,
         {
           struct StunServerList *ml;
 
-          /*Check if we do have a port*/
-          if((0 == pos_port) || (pos_port <= pos))
+          /* Check if we do have a port */
+          if ((0 == pos_port) || (pos_port <= pos))
           {
             LOG (GNUNET_ERROR_TYPE_WARNING,
                  "STUN server format mistake\n");
@@ -1677,7 +1679,7 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg,
           ml->port = atoi (&stun_servers[pos_port]);
 
           /* Remove trailing space */
-          if(stun_servers[pos] == ' ')
+          if (stun_servers[pos] == ' ')
             ml->address = GNUNET_strdup (&stun_servers[pos + 1]);
           else
             ml->address = GNUNET_strdup (&stun_servers[pos]);
@@ -1688,6 +1690,7 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg,
           GNUNET_CONTAINER_DLL_insert (h->stun_servers_head,
                                        h->stun_servers_tail,
                                        ml);
+          stun_servers[pos] = '\0';
         }
       }
     }
@@ -1767,9 +1770,30 @@ GNUNET_NAT_unregister (struct GNUNET_NAT_Handle *h)
   unsigned int i;
   struct LocalAddressList *lal;
   struct MiniList *ml;
-
+  struct StunServerList *ssl;
+  
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "NAT unregister called\n");
+  while (NULL != (ssl = h->stun_servers_head))
+  {
+    GNUNET_CONTAINER_DLL_remove (h->stun_servers_head,
+                                 h->stun_servers_tail,
+                                 ssl);
+    GNUNET_free (ssl->address);
+    GNUNET_free (ssl);
+  }
+  while (NULL != (lal = h->lal_head))
+  {
+    GNUNET_CONTAINER_DLL_remove (h->lal_head,
+                                 h->lal_tail,
+                                 lal);
+    if (NULL != h->address_callback)
+      h->address_callback (h->callback_cls,
+                           GNUNET_NO,
+                           (const struct sockaddr *) &lal[1],
+                           lal->addrlen);
+    GNUNET_free (lal);
+  }
   while (NULL != (ml = h->mini_head))
   {
     GNUNET_CONTAINER_DLL_remove (h->mini_head,
@@ -1839,18 +1863,6 @@ GNUNET_NAT_unregister (struct GNUNET_NAT_Handle *h)
     h->server_stdout = NULL;
     h->server_stdout_handle = NULL;
   }
-  while (NULL != (lal = h->lal_head))
-  {
-    GNUNET_CONTAINER_DLL_remove (h->lal_head,
-                                 h->lal_tail,
-                                 lal);
-    if (NULL != h->address_callback)
-      h->address_callback (h->callback_cls,
-                           GNUNET_NO,
-                           (const struct sockaddr *) &lal[1],
-                           lal->addrlen);
-    GNUNET_free (lal);
-  }
   for (i = 0; i < h->num_local_addrs; i++)
     GNUNET_free (h->local_addrs[i]);
   GNUNET_free_non_null (h->local_addrs);
diff --git a/src/nat/nat_stun.c b/src/nat/nat_stun.c
index 4aa9094ea..b914abb2e 100644
--- a/src/nat/nat_stun.c
+++ b/src/nat/nat_stun.c
@@ -461,9 +461,9 @@ stun_dns_callback (void *cls,
   int reqlen;
   struct sockaddr_in server;
 
-  rh->dns_active = NULL;
   if (NULL == addr)
   {
+    rh->dns_active = NULL;
     if (GNUNET_NO == rh->dns_success)
     {
       LOG (GNUNET_ERROR_TYPE_INFO,
@@ -471,8 +471,18 @@ stun_dns_callback (void *cls,
            rh->stun_server);
       rh->cb (rh->cb_cls,
               GNUNET_NAT_ERROR_NOT_ONLINE);
-      GNUNET_NAT_stun_make_request_cancel (rh);
     }
+    else if (GNUNET_SYSERR == rh->dns_success)
+    {
+      rh->cb (rh->cb_cls,
+              GNUNET_NAT_ERROR_INTERNAL_NETWORK_ERROR);
+    }
+    else
+    {
+      rh->cb (rh->cb_cls,
+              GNUNET_NAT_ERROR_SUCCESS);
+    }
+    GNUNET_NAT_stun_make_request_cancel (rh);
     return;
   }
 
@@ -487,31 +497,27 @@ stun_dns_callback (void *cls,
 
   /*Craft the simplest possible STUN packet. A request binding*/
   req = (struct stun_header *)reqdata;
-  generate_request_id(req);
+  generate_request_id (req);
   reqlen = 0;
   req->msgtype = 0;
   req->msglen = 0;
-  req->msglen = htons(reqlen);
-  req->msgtype = htons(encode_message(STUN_REQUEST, STUN_BINDING));
+  req->msglen = htons (reqlen);
+  req->msgtype = htons (encode_message (STUN_REQUEST,
+                                        STUN_BINDING));
 
   /* Send the packet */
-  if (-1 == GNUNET_NETWORK_socket_sendto (rh->sock,
-                                          req,
-                                          ntohs(req->msglen) + sizeof(*req),
-                                          (const struct sockaddr *) &server,
-                                          sizeof (server)))
+  if (-1 ==
+      GNUNET_NETWORK_socket_sendto (rh->sock,
+                                    req,
+                                    ntohs(req->msglen) + sizeof(*req),
+                                    (const struct sockaddr *) &server,
+                                    sizeof (server)))
   {
     GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR,
-                         "Fail to sendto");
-    rh->cb (rh->cb_cls,
-            GNUNET_NAT_ERROR_INTERNAL_NETWORK_ERROR);
-    GNUNET_NAT_stun_make_request_cancel (rh);
+                         "sendto");
+    rh->dns_success = GNUNET_SYSERR;
     return;
   }
-  /* sending STUN request done, let's wait for replies... */
-  rh->cb (rh->cb_cls,
-          GNUNET_NAT_ERROR_SUCCESS);
-  GNUNET_NAT_stun_make_request_cancel (rh);
 }
 
 
@@ -550,8 +556,6 @@ GNUNET_NAT_stun_make_request (const char *server,
                                            &stun_dns_callback, rh);
   if (NULL == rh->dns_active)
   {
-    GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR,
-                         "Failed DNS");
     GNUNET_NAT_stun_make_request_cancel (rh);
     return NULL;
   }
diff --git a/src/util/resolver_api.c b/src/util/resolver_api.c
index 299fdfef9..c4ce1ccc2 100644
--- a/src/util/resolver_api.c
+++ b/src/util/resolver_api.c
@@ -249,8 +249,16 @@ GNUNET_RESOLVER_connect (const struct GNUNET_CONFIGURATION_Handle *cfg)
 void
 GNUNET_RESOLVER_disconnect ()
 {
-  GNUNET_assert (NULL == req_head);
-  GNUNET_assert (NULL == req_tail);
+  struct GNUNET_RESOLVER_RequestHandle *rh;
+  
+  while (NULL != (rh = req_head))
+  {
+    GNUNET_assert (GNUNET_SYSERR == rh->was_transmitted);
+    GNUNET_CONTAINER_DLL_remove (req_head,
+                                 req_tail,
+                                 rh);
+    GNUNET_free (rh);
+  }
   if (NULL != mq)
   {
     LOG (GNUNET_ERROR_TYPE_DEBUG,
@@ -272,6 +280,42 @@ GNUNET_RESOLVER_disconnect ()
 
 
 /**
+ * Task executed on system shutdown.
+ */
+static void
+shutdown_task (void *cls)
+{
+  s_task = NULL;
+  GNUNET_RESOLVER_disconnect ();
+  backoff = GNUNET_TIME_UNIT_MILLISECONDS;
+}
+
+
+/**
+ * Consider disconnecting if we have no further requests pending.
+ */
+static void
+check_disconnect ()
+{
+  struct GNUNET_RESOLVER_RequestHandle *rh;
+  
+  for (rh = req_head; NULL != rh; rh = rh->next)
+    if (GNUNET_SYSERR != rh->was_transmitted)
+      return;
+  if (NULL != r_task)
+  {
+    GNUNET_SCHEDULER_cancel (r_task);
+    r_task = NULL;
+  }
+  if (NULL != s_task)
+    return;
+  s_task = GNUNET_SCHEDULER_add_delayed (GNUNET_TIME_UNIT_MILLISECONDS,
+                                         &shutdown_task,
+                                         NULL);
+}
+
+
+/**
  * Convert IP address to string without DNS resolution.
  *
  * @param af address family
@@ -339,23 +383,13 @@ mq_error_handler (void *cls,
 {
   GNUNET_MQ_destroy (mq);
   mq = NULL;
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "MQ error, reconnecting\n");
   reconnect ();
 }
 
 
 /**
- * Task executed on system shutdown.
- */
-static void
-shutdown_task (void *cls)
-{
-  s_task = NULL;
-  GNUNET_RESOLVER_disconnect ();
-  backoff = GNUNET_TIME_UNIT_MILLISECONDS;
-}
-
-
-/**
  * Process pending requests to the resolver.
  */
 static void
@@ -599,7 +633,9 @@ numeric_resolution (void *cls)
   }
   if ( ( (rh->af == AF_UNSPEC) ||
          (rh->af == AF_INET6) ) &&
-       (1 == inet_pton (AF_INET6, hostname, &v6.sin6_addr) ) )
+       (1 == inet_pton (AF_INET6,
+                        hostname,
+                        &v6.sin6_addr) ) )
   {
     rh->addr_callback (rh->cls,
                        (const struct sockaddr *) &v6,
@@ -671,6 +707,9 @@ loopback_resolution (void *cls)
   rh->addr_callback (rh->cls,
                      NULL,
                      0);
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Finished resolving hostname `%s'.\n",
+       (const char *) &rh[1]);
   GNUNET_free (rh);
 }
 
@@ -740,6 +779,7 @@ reconnect ()
                                    req_tail,
                                    rh);
       GNUNET_free (rh);
+      check_disconnect ();
       break;
     default:
       GNUNET_assert (0);
@@ -841,13 +881,16 @@ GNUNET_RESOLVER_ip_get (const char *hostname,
     GNUNET_break (0);
     return NULL;
   }
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Trying to resolve hostname `%s'.\n",
+       hostname);
   rh = GNUNET_malloc (sizeof (struct GNUNET_RESOLVER_RequestHandle) + slen);
   rh->af = af;
   rh->addr_callback = callback;
   rh->cls = callback_cls;
   GNUNET_memcpy (&rh[1],
-          hostname,
-          slen);
+                 hostname,
+                 slen);
   rh->data_len = slen;
   rh->timeout = GNUNET_TIME_relative_to_absolute (timeout);
   rh->direction = GNUNET_NO;
@@ -1133,6 +1176,10 @@ GNUNET_RESOLVER_hostname_resolve (int af,
 void
 GNUNET_RESOLVER_request_cancel (struct GNUNET_RESOLVER_RequestHandle *rh)
 {
+  if (GNUNET_NO == rh->direction)
+    LOG (GNUNET_ERROR_TYPE_DEBUG,
+         "Asked to cancel request to resolve hostname `%s'.\n",
+         (const char *) &rh[1]);
   if (NULL != rh->task)
   {
     GNUNET_SCHEDULER_cancel (rh->task);
@@ -1145,10 +1192,12 @@ GNUNET_RESOLVER_request_cancel (struct GNUNET_RESOLVER_RequestHandle *rh)
                                    req_tail,
                                    rh);
     GNUNET_free (rh);
+    check_disconnect ();
     return;
   }
   GNUNET_assert (GNUNET_YES == rh->was_transmitted);
   rh->was_transmitted = GNUNET_SYSERR;  /* mark as cancelled */
+  check_disconnect ();
 }