| Message ID | 20200119191833.581492-1-morganamilo@archlinux.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [pacman-dev] libalpm: free trans before databases | expand |
On 20/1/20 5:18 am, morganamilo wrote: > When releasing the handle, alpm tries to do some self clean up by > freeing the databases and transaction. > > However, databases refuse to unregister is there is an in progress > transaction. Causing them to leak if the handle is released while > a transaction is active. > So... I've been sitting on this for a while. It does not seem the right answer to the issue to me. But I am not sure what the right answer is! Suggestions inline below. > diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c > index 1a378db9..02b8fc9b 100644 > --- a/lib/libalpm/alpm.c > +++ b/lib/libalpm/alpm.c > @@ -30,6 +30,7 @@ > #include "alpm_list.h" > #include "handle.h" > #include "log.h" > +#include "trans.h" > #include "util.h" > > /** \addtogroup alpm_interface Interface Functions > @@ -114,6 +115,8 @@ int SYMEXPORT alpm_release(alpm_handle_t *myhandle) > > CHECK_HANDLE(myhandle, return -1); > > + _alpm_trans_free(myhandle->trans); > + Could we do an alpm_trans_release() here if handle->trans is non-null and abort if that fails? > /* close local database */ > db = myhandle->db_local; > if(db) { > diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c > index e3193f40..1e42f4a8 100644 > --- a/lib/libalpm/handle.c > +++ b/lib/libalpm/handle.c > @@ -34,7 +34,6 @@ > #include "alpm_list.h" > #include "util.h" > #include "log.h" > -#include "trans.h" > #include "alpm.h" > #include "deps.h" > > @@ -74,7 +73,6 @@ void _alpm_handle_free(alpm_handle_t *handle) > #endif > > /* free memory */ > - _alpm_trans_free(handle->trans); This needs to stay. It is unexpected that _alpm_handle_free does not free its whole memory. > FREE(handle->root); > FREE(handle->dbpath); > FREE(handle->dbext); >
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 1a378db9..02b8fc9b 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -30,6 +30,7 @@ #include "alpm_list.h" #include "handle.h" #include "log.h" +#include "trans.h" #include "util.h" /** \addtogroup alpm_interface Interface Functions @@ -114,6 +115,8 @@ int SYMEXPORT alpm_release(alpm_handle_t *myhandle) CHECK_HANDLE(myhandle, return -1); + _alpm_trans_free(myhandle->trans); + /* close local database */ db = myhandle->db_local; if(db) { diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index e3193f40..1e42f4a8 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -34,7 +34,6 @@ #include "alpm_list.h" #include "util.h" #include "log.h" -#include "trans.h" #include "alpm.h" #include "deps.h" @@ -74,7 +73,6 @@ void _alpm_handle_free(alpm_handle_t *handle) #endif /* free memory */ - _alpm_trans_free(handle->trans); FREE(handle->root); FREE(handle->dbpath); FREE(handle->dbext);