diff mbox series

[ovs-dev,v3,3/9] ovsdb-idl: Fix memory leak sending messages without a session.

Message ID 20201218053144.2637583-4-blp@ovn.org
State Accepted
Headers show
Series Refactor OVSDB IDL into two layers | expand

Commit Message

Ben Pfaff Dec. 18, 2020, 5:31 a.m. UTC
When there's no open session, we still have to free the messages that
we make but cannot send.

I'm not confident that these fix actual bugs, because it seems possible
that these code paths can only be hit when the session is nonnull.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 lib/ovsdb-idl.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Ilya Maximets Dec. 18, 2020, 9:37 p.m. UTC | #1
On 12/18/20 6:31 AM, Ben Pfaff wrote:
> When there's no open session, we still have to free the messages that
> we make but cannot send.
> 
> I'm not confident that these fix actual bugs, because it seems possible
> that these code paths can only be hit when the session is nonnull.
> 
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>  lib/ovsdb-idl.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 

I'm not sure if this fixes actual bugs too.  And it seems like we
could have more serious issues if we'll ever hit that code,
but at least there will be less memory leaks. :)  So,

Acked-by: Ilya Maximets <i.maximets@ovn.org>

> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index fb638c499c51..02a49b32454b 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -710,6 +710,8 @@ ovsdb_idl_send_request(struct ovsdb_idl *idl, struct jsonrpc_msg *request)
>      idl->request_id = json_clone(request->id);
>      if (idl->session) {
>          jsonrpc_session_send(idl->session, request);
> +    } else {
> +        jsonrpc_msg_destroy(request);
>      }
>  }
>  
> @@ -4489,8 +4491,10 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
>      if (!any_updates) {
>          txn->status = TXN_UNCHANGED;
>          json_destroy(operations);
> -    } else if (txn->db->idl->session
> -               && !jsonrpc_session_send(
> +    } else if (!txn->db->idl->session) {
> +        txn->status = TXN_TRY_AGAIN;
> +        json_destroy(operations);
> +    } else if (!jsonrpc_session_send(
>                     txn->db->idl->session,
>                     jsonrpc_create_request(
>                         "transact", operations, &txn->request_id))) {
> @@ -5198,6 +5202,8 @@ ovsdb_idl_set_lock(struct ovsdb_idl *idl, const char *lock_name)
>          }
>          if (idl->session) {
>              jsonrpc_session_send(idl->session, msg);
> +        } else {
> +            jsonrpc_msg_destroy(msg);
>          }
>      }
>  }
>
diff mbox series

Patch

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index fb638c499c51..02a49b32454b 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -710,6 +710,8 @@  ovsdb_idl_send_request(struct ovsdb_idl *idl, struct jsonrpc_msg *request)
     idl->request_id = json_clone(request->id);
     if (idl->session) {
         jsonrpc_session_send(idl->session, request);
+    } else {
+        jsonrpc_msg_destroy(request);
     }
 }
 
@@ -4489,8 +4491,10 @@  ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
     if (!any_updates) {
         txn->status = TXN_UNCHANGED;
         json_destroy(operations);
-    } else if (txn->db->idl->session
-               && !jsonrpc_session_send(
+    } else if (!txn->db->idl->session) {
+        txn->status = TXN_TRY_AGAIN;
+        json_destroy(operations);
+    } else if (!jsonrpc_session_send(
                    txn->db->idl->session,
                    jsonrpc_create_request(
                        "transact", operations, &txn->request_id))) {
@@ -5198,6 +5202,8 @@  ovsdb_idl_set_lock(struct ovsdb_idl *idl, const char *lock_name)
         }
         if (idl->session) {
             jsonrpc_session_send(idl->session, msg);
+        } else {
+            jsonrpc_msg_destroy(msg);
         }
     }
 }