| Message ID | 1465137455-8804-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
"dev" <dev-bounces@openvswitch.org> wrote on 06/05/2016 09:37:35 AM: > From: William Tu <u9012063@gmail.com> > To: dev@openvswitch.org > Date: 06/05/2016 09:37 AM > Subject: [ovs-dev] [PATCH] ovn-controller: Fix memory leak reported > by valgrind. > Sent by: "dev" <dev-bounces@openvswitch.org> > > Calling ovsdb_idl_set_remote() might overwrite the 'idl->session'. The patch > fixes them by freeing 'idl->session' before it is overwritten. > > Testcast ovn-controller - ovn-bridge-mappings reports two definitelylosts in: > xmalloc (util.c:112) > jsonrpc_session_open (jsonrpc.c:784) > ovsdb_idl_create (ovsdb-idl.c:246) > main (ovn-controller.c:384) > and, > xmalloc (util.c:112) > jsonrpc_session_open (jsonrpc.c:784) > ovsdb_idl_set_remote (ovsdb-idl.c:289) > main (ovn-controller.c:409) > > Signed-off-by: William Tu <u9012063@gmail.com> Looked at this by inspection and it makes sense. I also checked ovsdb/jsonrpc-server.c but I don't think that usage leaks - William, did you also check that? Acked-by: Ryan Moats <rmoats@us.ibm.com>
Hi Ryan, Thanks for the review. I looked at the jsonrpc-server.c and I think it's OK without leaks. Regards, William On Wed, Jun 8, 2016 at 7:32 PM, Ryan Moats <rmoats@us.ibm.com> wrote: > "dev" <dev-bounces@openvswitch.org> wrote on 06/05/2016 09:37:35 AM: > >> From: William Tu <u9012063@gmail.com> >> To: dev@openvswitch.org >> Date: 06/05/2016 09:37 AM >> Subject: [ovs-dev] [PATCH] ovn-controller: Fix memory leak reported >> by valgrind. >> Sent by: "dev" <dev-bounces@openvswitch.org> >> >> Calling ovsdb_idl_set_remote() might overwrite the 'idl->session'. The >> patch >> fixes them by freeing 'idl->session' before it is overwritten. >> >> Testcast ovn-controller - ovn-bridge-mappings reports two definitelylosts >> in: >> xmalloc (util.c:112) >> jsonrpc_session_open (jsonrpc.c:784) >> ovsdb_idl_create (ovsdb-idl.c:246) >> main (ovn-controller.c:384) >> and, >> xmalloc (util.c:112) >> jsonrpc_session_open (jsonrpc.c:784) >> ovsdb_idl_set_remote (ovsdb-idl.c:289) >> main (ovn-controller.c:409) >> >> Signed-off-by: William Tu <u9012063@gmail.com> > > Looked at this by inspection and it makes sense. I also checked > ovsdb/jsonrpc-server.c but I don't think that usage leaks - William, did you > also check that? > > Acked-by: Ryan Moats <rmoats@us.ibm.com>
On Sun, Jun 05, 2016 at 07:37:35AM -0700, William Tu wrote: > Calling ovsdb_idl_set_remote() might overwrite the 'idl->session'. The patch > fixes them by freeing 'idl->session' before it is overwritten. > > Testcast ovn-controller - ovn-bridge-mappings reports two definitely losts in: > xmalloc (util.c:112) > jsonrpc_session_open (jsonrpc.c:784) > ovsdb_idl_create (ovsdb-idl.c:246) > main (ovn-controller.c:384) > and, > xmalloc (util.c:112) > jsonrpc_session_open (jsonrpc.c:784) > ovsdb_idl_set_remote (ovsdb-idl.c:289) > main (ovn-controller.c:409) > > Signed-off-by: William Tu <u9012063@gmail.com> > --- > lib/ovsdb-idl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 2b372cb..15421ac 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -286,6 +286,9 @@ ovsdb_idl_set_remote(struct ovsdb_idl *idl, const char *remote, > { > if (idl) { > ovs_assert(!idl->txn); > + if (idl->session) { > + jsonrpc_session_close(idl->session); > + } Thanks, I applied this to master. I removed the "if" because jsonrpc_session_close() is a no-op for null pointers.
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 2b372cb..15421ac 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -286,6 +286,9 @@ ovsdb_idl_set_remote(struct ovsdb_idl *idl, const char *remote, { if (idl) { ovs_assert(!idl->txn); + if (idl->session) { + jsonrpc_session_close(idl->session); + } idl->session = jsonrpc_session_open(remote, retry); idl->state_seqno = UINT_MAX; }
Calling ovsdb_idl_set_remote() might overwrite the 'idl->session'. The patch fixes them by freeing 'idl->session' before it is overwritten. Testcast ovn-controller - ovn-bridge-mappings reports two definitely losts in: xmalloc (util.c:112) jsonrpc_session_open (jsonrpc.c:784) ovsdb_idl_create (ovsdb-idl.c:246) main (ovn-controller.c:384) and, xmalloc (util.c:112) jsonrpc_session_open (jsonrpc.c:784) ovsdb_idl_set_remote (ovsdb-idl.c:289) main (ovn-controller.c:409) Signed-off-by: William Tu <u9012063@gmail.com> --- lib/ovsdb-idl.c | 3 +++ 1 file changed, 3 insertions(+)