Commit 3b18b0d6 authored by andy zhou's avatar andy zhou Committed by Andy Zhou

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: default avatarLei Huang <huang.f.lei@gmail.com>
Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.htmlSigned-off-by: default avatarAndy Zhou <azhou@ovn.org>
Tested-by: default avatarHan Zhou <zhouhan@gmail.com>
Acked-by: default avatarHan Zhou <zhouhan@gmail.com>
Acked-by: default avatarLiran Schour <lirans@il.ibm.com>
parent 23fba242
......@@ -103,6 +103,7 @@ Krishna Kondaka kkondaka@vmware.com
Kyle Mestery mestery@mestery.com
Kyle Upton kupton@baymicrosystems.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
......
......@@ -1238,7 +1238,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;
}
......@@ -1316,7 +1316,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);
}
......
......@@ -763,7 +763,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;
......@@ -775,6 +776,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);
......
......@@ -36,10 +36,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);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment