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

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index b1874315972938befe0f1b0ac7f54b9d48a4f8e3..4112e808ad7758475660e69e1af3d27a7f95b495 100644 (file)
--- 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 3f26f0f620ae888c29b44453be01c51d390cc3af..3827fb436adc55767f9ffd830419842eb5174b6b 100644 (file)
--- 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 477bbb8c6ea3fce23fd82b8b421beff0923909e4..6760b27cbab6e2baffe459d675b56be487cde233 100644 (file)
--- 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 14066462b84a6a7b4f49d91f3531d51dba189132..8b5211ed5a475254352dd40fe1028011def384de 100644 (file)
--- 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 046e62e1785bb9eb799a5b78881eb5847fcc6900..907efb242800852023e36211bcfe9baf3bc39d6c 100644 (file)
--- 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 22eb7c71200582f6a3946273b89d50ca8ae902b7..c0ecb99ed5ebfca661d3e1288e96083a1f41d612 100644 (file)
--- 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 5e8099647959e7071e0dafc28559dc346ad79d85..1150ec1b068ad6fcb3ae51b49b94be3ebfc23cea 100644 (file)
--- 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 b8f5bde70e3da9874749a2e4be01b6f9806efe09..69d45950ae7261770f3ad514b155687fa220af93 100644 (file)
--- 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;
     }