389-ds-base

Clone
Source Code
GIT

#51021 Issue 51016 - Fix memory leaks in changelog5_init and perfctrs_init
Closed 3 years ago by spichugi. Opened 4 years ago by spichugi.
spichugi/389-ds-base i51016  into  master

file modified
+1 -1 
@@ -338,7 +338,7 @@ 
 

  

      if (slapi_pblock_set(pb, SLAPI_PLUGIN_VERSION, SLAPI_PLUGIN_VERSION_01) != 0 ||
 

          slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void *)&multimasterbepreopdesc) != 0 ||
 

-         slapi_pblock_set(pb, SLAPI_PLUGIN_BE_PRE_CLOSE_FN, (void *)cl5Close) != 0 ||
 

+         slapi_pblock_set(pb, SLAPI_PLUGIN_BE_PRE_CLOSE_FN, (void *)cl5Cleanup) != 0 ||
 

          slapi_pblock_set(pb, SLAPI_PLUGIN_BE_PRE_BACKUP_FN, (void *)cl5WriteRUV) != 0) {
 

          slapi_log_err(SLAPI_LOG_PLUGIN, repl_plugin_name, "multimaster_bepreop_init - Failed\n");
 

          rc = -1;

file modified
+8 
@@ -1293,6 +1293,14 @@ 
 

                  return return_value;
 

              }
 

  

+             /* We need to free the memory to avoid a leak
 

+              * Also, we have to evaluate if the performance counter
 

+              * should be preserved or not for database restore.
 

+              * Look - https://pagure.io/389-ds-base/issue/51020
 

+              */
 

+             if (conf->perf_private) {
 

+                 perfctrs_terminate(&conf->perf_private, pEnv->bdb_DB_ENV);
 

+             }
 

              /* Now open the performance counters stuff */
 

              perfctrs_init(li, &(conf->perf_private));
 

              if (getenv(TXN_TESTING)) {

Bug Description: Memory Leaks are detected by ASAN in changelog5_init
and perfctrs_init functions.

Fix Description: Free existing memory before initializing new memory
which will be assigned to the existing stucts.

https://pagure.io/389-ds-base/issue/51016

Reviewed by: ?

rebased onto 7e61d4bacc99db4cf1c2e5ef34cbb6845fb33dd1
4 years ago

I don't like the call to cl5Cleanup in chanelog5_init. It might fix the leak, but doing the cleanup before the init is not the right place, it is normally used only at startup and the you do not need the cleanup.
Only in the archive case where changelog5_init is registered for SLAPI_PLUGIN_BE_POST_OPEN_FN it can cause problems, but this means that it was not properly cleaned when closed. You should ensur that the function registered for SLAPI_PLUGIN_BE_PRE_CLOSE_FN does the right thing, it should probably be changelog 5_cleanup instead of cl5Close

rebased onto b6571896505432fa98e23115367066a688c23124
4 years ago

Changes are made and tested for the leaks (it's clean). Please, review :)

Ack

@lkrispen did you suggest to call changelog5_cleanup or cl5Cleanup, during SLAPI_PLUGIN_BE_PRE_CLOSE_FN ?

cl5Cleanup also calls cl5Close, changelog5_cleanup calls both (and in addition some config cleanup, but looks like this doesn't leak). So I am fine with cl5Cleanup

rebased onto 46fb7ce
4 years ago

Pull-Request has been merged by spichugi
4 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4074

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 2
+1 -1
file changed
ldap/servers/plugins/replication/repl5_init.c
+8 -0
file changed
ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.