[ovs-dev] [PATCH] ovsdb-server: Fix a reference count leak bug

Andy Zhou azhou at ovn.org
Mon Mar 7 23:44:34 UTC 2016


When destroying an ovsdb_jsonrpc_monitor, the jsonrpc monitor still
holds a reference count to the monitors 'changes' indexed with
'unflushed' transaction id.  The bug is that the reference count was
not decremented as it should in the code path.

The bug caused 'changes' that have been flushed to all jsonrpc
clients to linger around unnecessarily, occupying increasingly
large amount of memory. See "Reported-at" URL for more details.

This bug is tricky to find since the memory is not leaked; they will
eventually be freed when monitors are destroyed.

Reported-by: Lei Huang <huang.f.lei at gmail.com>
Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html
Signed-off-by: Andy Zhou <azhou at ovn.org>
---
 AUTHORS                | 1 +
 ovsdb/jsonrpc-server.c | 4 ++--
 ovsdb/monitor.c        | 9 ++++++++-
 ovsdb/monitor.h        | 6 ++----
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 1184a51..0ba0f58 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -119,6 +119,7 @@ Kyle Mestery            mestery at mestery.com
 Kyle Upton              kupton at baymicrosystems.com
 Lance Richardson        lrichard at redhat.com
 Lars Kellogg-Stedman    lars at redhat.com
+Lei Huang               huang.f.lei at gmail.com
 Leo Alterman            lalterman at nicira.com
 Lilijun                 jerry.lilijun at huawei.com
 Linda Sun               lsun at vmware.com
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 15dbc4e..caaf2bf 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1265,7 +1265,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db,
     dbmon = ovsdb_monitor_add(m->dbmon);
     if (dbmon != m->dbmon) {
         /* Found an exisiting dbmon, reuse the current one. */
-        ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
+        ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, m->unflushed);
         ovsdb_monitor_add_jsonrpc_monitor(dbmon, m);
         m->dbmon = dbmon;
     }
@@ -1348,7 +1348,7 @@ ovsdb_jsonrpc_monitor_destroy(struct ovsdb_jsonrpc_monitor *m)
 {
     json_destroy(m->monitor_id);
     hmap_remove(&m->session->monitors, &m->node);
-    ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
+    ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, m->unflushed);
     free(m);
 }
 
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 0d81d89..6b0d13d 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -959,7 +959,8 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
 
 void
 ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
-                   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
+                   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
+                   uint64_t unflushed)
 {
     struct jsonrpc_monitor_node *jm;
 
@@ -971,6 +972,12 @@ ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
     /* Find and remove the jsonrpc monitor from the list.  */
     LIST_FOR_EACH(jm, node, &dbmon->jsonrpc_monitors) {
         if (jm->jsonrpc_monitor == jsonrpc_monitor) {
+            /* Release the tracked changes. */
+            struct shash_node *node;
+            SHASH_FOR_EACH (node, &dbmon->tables) {
+                struct ovsdb_monitor_table *mt = node->data;
+                ovsdb_monitor_table_untrack_changes(mt, unflushed);
+            }
             list_remove(&jm->node);
             free(jm);
 
diff --git a/ovsdb/monitor.h b/ovsdb/monitor.h
index fb10435..9fea831 100644
--- a/ovsdb/monitor.h
+++ b/ovsdb/monitor.h
@@ -46,10 +46,8 @@ void ovsdb_monitor_add_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
                        struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
 
 void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
-                       struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
-
-void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
-                               struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
+                               struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
+                               uint64_t unflushed);
 
 void ovsdb_monitor_add_table(struct ovsdb_monitor *m,
                              const struct ovsdb_table *table);
-- 
1.9.1




More information about the dev mailing list