[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