ovsdb-server: Fix a reference count leak bug

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@gmail.com>
Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html
Signed-off-by: Andy Zhou <azhou@ovn.org>
Tested-by: Han Zhou <zhouhan@gmail.com>
Acked-by: Han Zhou <zhouhan@gmail.com>
Acked-by: Liran Schour <lirans@il.ibm.com>

diff --git a/AUTHORS b/AUTHORS
index 1184a51..0ba0f58 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -119,6 +119,7 @@ Kyle Mestery            mestery@mestery.com
 Kyle Upton              kupton@baymicrosystems.com
 Lance Richardson        lrichard@redhat.com
 Lars Kellogg-Stedman    lars@redhat.com
+Lei Huang               huang.f.lei@gmail.com
 Leo Alterman            lalterman@nicira.com
 Lilijun                 jerry.lilijun@huawei.com
 Linda Sun               lsun@vmware.com

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 15dbc4e..caaf2bf 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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);