[ovs-dev] [PATCH] ovsdb-server: Fix a reference count leak bug
Liran Schour
LIRANS at il.ibm.com
Tue Mar 8 06:46:21 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
>
Acked-by: Liran Schour <lirans at il.ibm.com>
More information about the dev
mailing list