ares_cancel(): cancel requests safely

An invocation of ares_cancel() walks through the request list, calling
the callbacks of all pending requests on a channel. Previously, if such
a callback added a new request to the channel, the request list might
not end up empty, causing an abort by assertion failure. The present
commit ensures that precisely all requests present upon entry of
ares_cancel() are cancelled, and that adding new requests through
callbacks is safe.

diff --git a/ares_cancel.c b/ares_cancel.c
index e5bb050..465cc9e 100644 (file)
--- a/ares_cancel.c
+++ b/ares_cancel.c
@@ -26,33 +26,33 @@
 void ares_cancel(ares_channel channel)
 {
   struct query *query;
+  struct list_node list_head_copy;
   struct list_node* list_head;
   struct list_node* list_node;
   int i;
 
-  list_head = &(channel->all_queries);
-  for (list_node = list_head->next; list_node != list_head; )
+  if (!ares__is_list_empty(&(channel->all_queries)))
   {
-    query = list_node->data;
-    list_node = list_node->next;  /* since we're deleting the query */
-    query->callback(query->arg, ARES_ECANCELLED, 0, NULL, 0);
-    ares__free_query(query);
-  }
-#ifndef NDEBUG
-  /* Freeing the query should remove it from all the lists in which it sits,
-   * so all query lists should be empty now.
-   */
-  assert(ares__is_list_empty(&(channel->all_queries)));
-  for (i = 0; i < ARES_QID_TABLE_SIZE; i++)
-    {
-      assert(ares__is_list_empty(&(channel->queries_by_qid[i])));
-    }
-  for (i = 0; i < ARES_TIMEOUT_TABLE_SIZE; i++)
+    /* Swap list heads, so that only those queries which were present on entry
+     * into this function are cancelled. New queries added by callbacks of
+     * queries being cancelled will not be cancelled themselves.
+     */
+    list_head = &(channel->all_queries);
+    list_head_copy.prev = list_head->prev;
+    list_head_copy.next = list_head->next;
+    list_head_copy.prev->next = &list_head_copy;
+    list_head_copy.next->prev = &list_head_copy;
+    list_head->prev = list_head;
+    list_head->next = list_head;
+    for (list_node = list_head_copy.next; list_node != &list_head_copy; )
     {
-      assert(ares__is_list_empty(&(channel->queries_by_timeout[i])));
+      query = list_node->data;
+      list_node = list_node->next;  /* since we're deleting the query */
+      query->callback(query->arg, ARES_ECANCELLED, 0, NULL, 0);
+      ares__free_query(query);
     }
-#endif
-  if (!(channel->flags & ARES_FLAG_STAYOPEN))
+  }
+  if (!(channel->flags & ARES_FLAG_STAYOPEN) && ares__is_list_empty(&(channel->all_queries)))
   {
     if (channel->servers)
     {