From 495f0b13e1b4b649368c970b139cc7fbdbefda84 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Wed, 2 Sep 2015 05:19:44 -0700
Subject: [PATCH] Fix some more memory issues in Quagga

Ticket: CM-4109
Reviewed-by: CCR-3414
Testing: See bug

Fixup of these memory issues:

(A) peer->clear_node_queue was accidently removed.  Add back in.
(B) Clean up bm->process_main_queue and bm->process_rsclient_queue initialization
(C) Some memory leaks
(D) Clean up unused threads
---
 bgpd/bgp_fsm.c        |  4 ----
 bgpd/bgp_route.c      | 27 +++++++++++------------
 bgpd/bgp_updgrp.c     |  5 +++++
 bgpd/bgpd.c           |  6 ++++++
 lib/thread.c          | 50 +++++++++++++++++++++++++++++--------------
 lib/thread.h          |  1 +
 zebra/if_sysctl.c     |  1 +
 zebra/rtread_sysctl.c |  1 +
 8 files changed, 61 insertions(+), 34 deletions(-)

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index b187431597..4112e808ad 100644
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -857,10 +857,6 @@ bgp_update_delay_begin (struct bgp *bgp)
   struct listnode *node, *nnode;
   struct peer *peer;
 
-  if ((bm->process_main_queue == NULL) ||
-      (bm->process_rsclient_queue == NULL))
-    bgp_process_queue_init();
-
   /* Stop the processing of queued work. Enqueue shall continue */
   work_queue_plug(bm->process_main_queue);
   work_queue_plug(bm->process_rsclient_queue);
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 3f26f0f620..3827fb436a 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2174,11 +2174,18 @@ bgp_processq_del (struct work_queue *wq, void *data)
 void
 bgp_process_queue_init (void)
 {
-  bm->process_main_queue
-    = work_queue_new (bm->master, "process_main_queue");
-  bm->process_rsclient_queue
-    = work_queue_new (bm->master, "process_rsclient_queue");
-  
+  if (!bm->process_main_queue)
+    {
+      bm->process_main_queue
+	= work_queue_new (bm->master, "process_main_queue");
+    }
+
+  if (!bm->process_rsclient_queue)
+    {
+      bm->process_rsclient_queue
+	= work_queue_new (bm->master, "process_rsclient_queue");
+    }
+
   if ( !(bm->process_main_queue && bm->process_rsclient_queue) )
     {
       zlog_err ("%s: Failed to allocate work queue", __func__);
@@ -2205,11 +2212,7 @@ bgp_process (struct bgp *bgp, struct bgp_node *rn, afi_t afi, safi_t safi)
   /* already scheduled for processing? */
   if (CHECK_FLAG (rn->flags, BGP_NODE_PROCESS_SCHEDULED))
     return;
-  
-  if ( (bm->process_main_queue == NULL) ||
-       (bm->process_rsclient_queue == NULL) )
-    bgp_process_queue_init ();
-  
+
   pqnode = XCALLOC (MTYPE_BGP_PROCESS_QUEUE, 
                     sizeof (struct bgp_process_queue));
   if (!pqnode)
@@ -2242,10 +2245,6 @@ bgp_add_eoiu_mark (struct bgp *bgp, bgp_table_t type)
 {
   struct bgp_process_queue *pqnode;
 
-  if ( (bm->process_main_queue == NULL) ||
-       (bm->process_rsclient_queue == NULL) )
-    bgp_process_queue_init ();
-
   pqnode = XCALLOC (MTYPE_BGP_PROCESS_QUEUE,
                     sizeof (struct bgp_process_queue));
   if (!pqnode)
diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c
index 477bbb8c6e..6760b27cba 100644
--- a/bgpd/bgp_updgrp.c
+++ b/bgpd/bgp_updgrp.c
@@ -223,6 +223,10 @@ conf_release (struct peer *src, afi_t afi, safi_t safi)
 
   if (srcfilter->usmap.name)
     XFREE(MTYPE_BGP_FILTER_NAME, srcfilter->usmap.name);
+
+  if (src->host)
+    XFREE(MTYPE_BGP_PEER_HOST, src->host);
+  src->host = NULL;
 }
 
 static void
@@ -717,6 +721,7 @@ update_group_create (struct peer_af *paf)
 
   UPDGRP_GLOBAL_STAT (updgrp, updgrps_created) += 1;
 
+  conf_release(&tmp_conf, paf->afi, paf->safi);
   return updgrp;
 }
 
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 14066462b8..8b5211ed5a 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -955,6 +955,9 @@ peer_free (struct peer *peer)
   if (peer->notify.data)
     XFREE(MTYPE_TMP, peer->notify.data);
 
+  if (peer->clear_node_queue)
+    work_queue_free(peer->clear_node_queue);
+
   bgp_sync_delete (peer);
 
   if (peer->conf_if)
@@ -2895,6 +2898,7 @@ bgp_delete (struct bgp *bgp)
   if (list_isempty(bm->bgp))
     bgp_close ();
 
+  thread_master_free_unused(bm->master);
   bgp_unlock(bgp);  /* initial reference */
 
   return 0;
@@ -6833,6 +6837,8 @@ bgp_master_init (void)
   bm->port = BGP_PORT_DEFAULT;
   bm->master = thread_master_create ();
   bm->start_time = bgp_clock ();
+
+  bgp_process_queue_init();
 }
 
 
diff --git a/lib/thread.c b/lib/thread.c
index 046e62e178..907efb2428 100644
--- a/lib/thread.c
+++ b/lib/thread.c
@@ -607,6 +607,22 @@ thread_add_fd (struct thread **thread_array, struct thread *thread)
   thread_array[thread->u.fd] = thread;
 }
 
+/* Thread list is empty or not.  */
+static int
+thread_empty (struct thread_list *list)
+{
+  return  list->head ? 0 : 1;
+}
+
+/* Delete top of the list and return it. */
+static struct thread *
+thread_trim_head (struct thread_list *list)
+{
+  if (!thread_empty (list))
+    return thread_list_delete (list, list->head);
+  return NULL;
+}
+
 /* Move thread to unuse list. */
 static void
 thread_add_unuse (struct thread_master *m, struct thread *thread)
@@ -666,6 +682,24 @@ thread_queue_free (struct thread_master *m, struct pqueue *queue)
   pqueue_delete(queue);
 }
 
+/*
+ * thread_master_free_unused
+ *
+ * As threads are finished with they are put on the
+ * unuse list for later reuse.
+ * If we are shutting down, Free up unused threads
+ * So we can see if we forget to shut anything off
+ */
+void
+thread_master_free_unused (struct thread_master *m)
+{
+  struct thread *t;
+  while ((t = thread_trim_head(&m->unuse)) != NULL)
+    {
+      XFREE(MTYPE_THREAD, t);
+    }
+}
+
 /* Stop thread scheduler. */
 void
 thread_master_free (struct thread_master *m)
@@ -688,22 +722,6 @@ thread_master_free (struct thread_master *m)
     }
 }
 
-/* Thread list is empty or not.  */
-static int
-thread_empty (struct thread_list *list)
-{
-  return  list->head ? 0 : 1;
-}
-
-/* Delete top of the list and return it. */
-static struct thread *
-thread_trim_head (struct thread_list *list)
-{
-  if (!thread_empty (list))
-    return thread_list_delete (list, list->head);
-  return NULL;
-}
-
 /* Return remain time in second. */
 unsigned long
 thread_timer_remain_second (struct thread *thread)
diff --git a/lib/thread.h b/lib/thread.h
index 22eb7c7120..c0ecb99ed5 100644
--- a/lib/thread.h
+++ b/lib/thread.h
@@ -183,6 +183,7 @@ enum quagga_clkid {
 /* Prototypes. */
 extern struct thread_master *thread_master_create (void);
 extern void thread_master_free (struct thread_master *);
+extern void thread_master_free_unused(struct thread_master *);
 
 extern struct thread *funcname_thread_add_read (struct thread_master *, 
 				                int (*)(struct thread *),
diff --git a/zebra/if_sysctl.c b/zebra/if_sysctl.c
index 5e80996479..1150ec1b06 100644
--- a/zebra/if_sysctl.c
+++ b/zebra/if_sysctl.c
@@ -66,6 +66,7 @@ ifstat_update_sysctl (void)
   if (sysctl (mib, MIBSIZ, buf, &bufsiz, NULL, 0) < 0) 
     {
       zlog (NULL, LOG_WARNING, "sysctl error by %s", safe_strerror (errno));
+      XFREE(MTYPE_TMP, ref);
       return;
     }
 
diff --git a/zebra/rtread_sysctl.c b/zebra/rtread_sysctl.c
index b8f5bde70e..69d45950ae 100644
--- a/zebra/rtread_sysctl.c
+++ b/zebra/rtread_sysctl.c
@@ -62,6 +62,7 @@ route_read (void)
   if (sysctl (mib, MIBSIZ, buf, &bufsiz, NULL, 0) < 0) 
     {
       zlog_warn ("sysctl() fail by %s", safe_strerror (errno));
+      XFREE(MTYPE_TMP, ref);
       return;
     }
 
-- 
2.39.5