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....@gmail.com> Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html Signed-off-by: Andy Zhou <az...@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 mest...@mestery.com Kyle Upton kup...@baymicrosystems.com Lance Richardson lrich...@redhat.com Lars Kellogg-Stedman l...@redhat.com +Lei Huang huang.f....@gmail.com Leo Alterman lalter...@nicira.com Lilijun jerry.lili...@huawei.com Linda Sun l...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
