From: andy zhou Date: Mon, 7 Mar 2016 23:44:34 +0000 (-0800) Subject: ovsdb-server: Fix a reference count leak bug X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=f76def2592cc5cb449a3360430ee9cc0f208765d 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 Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html Signed-off-by: Andy Zhou Tested-by: Han Zhou Acked-by: Han Zhou Acked-by: Liran Schour --- diff --git a/AUTHORS b/AUTHORS index 1184a5157..0ba0f58b8 100644 --- 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 15dbc4e74..caaf2bf49 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 0d81d89b6..6b0d13d67 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 fb1043541..9fea83130 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);