389-ds-base

Clone
Source Code
GIT

#51148 Issue 50984 - Memory leaks in disk monitoring
Closed 3 years ago by spichugi. Opened 3 years ago by spichugi.
spichugi/389-ds-base i50984  into  master

file modified
+41 -34 
@@ -885,6 +885,46 @@ 
 

  }
 

  

  void
 

+ slapd_sockets_ports_free(daemon_ports_t *ports_info)
 

+ {
 

+     /* freeing PRFileDescs */
 

+     PRFileDesc **fdesp = NULL;
 

+     for (fdesp = ports_info->n_socket; fdesp && *fdesp; fdesp++) {
 

+         PR_Close(*fdesp);
 

+     }
 

+     slapi_ch_free((void **)&ports_info->n_socket);
 

+ 

+     for (fdesp = ports_info->s_socket; fdesp && *fdesp; fdesp++) {
 

+         PR_Close(*fdesp);
 

+     }
 

+     slapi_ch_free((void **)&ports_info->s_socket);
 

+ #if defined(ENABLE_LDAPI)
 

+     for (fdesp = ports_info->i_socket; fdesp && *fdesp; fdesp++) {
 

+         PR_Close(*fdesp);
 

+     }
 

+     slapi_ch_free((void **)&ports_info->i_socket);
 

+ #endif /* ENABLE_LDAPI */
 

+ 

+     /* freeing NetAddrs */
 

+     PRNetAddr **nap;
 

+     for (nap = ports_info->n_listenaddr; nap && *nap; nap++) {
 

+         slapi_ch_free((void **)nap);
 

+     }
 

+     slapi_ch_free((void **)&ports_info->n_listenaddr);
 

+ 

+     for (nap = ports_info->s_listenaddr; nap && *nap; nap++) {
 

+         slapi_ch_free((void **)nap);
 

+     }
 

+     slapi_ch_free((void **)&ports_info->s_listenaddr);
 

+ #if defined(ENABLE_LDAPI)
 

+     for (nap = ports_info->i_listenaddr; nap && *nap; nap++) {
 

+         slapi_ch_free((void **)nap);
 

+     }
 

+     slapi_ch_free((void **)&ports_info->i_listenaddr);
 

+ #endif
 

+ }
 

+ 

+ void
 

  slapd_daemon(daemon_ports_t *ports)
 

  {
 

      /* We are passed some ports---one for regular connections, one
 
@@ -1099,40 +1139,7 @@ 
 

      /* free the listener indexes */
 

      slapi_ch_free((void **)&listener_idxs);
 

  

-     for (fdesp = n_tcps; fdesp && *fdesp; fdesp++) {
 

-         PR_Close(*fdesp);
 

-     }
 

-     slapi_ch_free((void **)&n_tcps);
 

- 

-     for (fdesp = i_unix; fdesp && *fdesp; fdesp++) {
 

-         PR_Close(*fdesp);
 

-     }
 

-     slapi_ch_free((void **)&i_unix);
 

- 

-     for (fdesp = s_tcps; fdesp && *fdesp; fdesp++) {
 

-         PR_Close(*fdesp);
 

-     }
 

-     slapi_ch_free((void **)&s_tcps);
 

- 

-     /* freeing NetAddrs */
 

-     {
 

-         PRNetAddr **nap;
 

-         for (nap = ports->n_listenaddr; nap && *nap; nap++) {
 

-             slapi_ch_free((void **)nap);
 

-         }
 

-         slapi_ch_free((void **)&ports->n_listenaddr);
 

- 

-         for (nap = ports->s_listenaddr; nap && *nap; nap++) {
 

-             slapi_ch_free((void **)nap);
 

-         }
 

-         slapi_ch_free((void **)&ports->s_listenaddr);
 

- #if defined(ENABLE_LDAPI)
 

-         for (nap = ports->i_listenaddr; nap && *nap; nap++) {
 

-             slapi_ch_free((void **)nap);
 

-         }
 

-         slapi_ch_free((void **)&ports->i_listenaddr);
 

- #endif
 

-     }
 

+     slapd_sockets_ports_free(ports);
 

  

      op_thread_cleanup();
 

      housekeeping_stop(); /* Run this after op_thread_cleanup() logged sth */

file modified
+1 
@@ -120,6 +120,7 @@ 
 

   */
 

  int signal_listner(void);
 

  int daemon_pre_setuid_init(daemon_ports_t *ports);
 

+ void slapd_sockets_ports_free(daemon_ports_t *ports_info);
 

  void slapd_daemon(daemon_ports_t *ports);
 

  void daemon_register_connection(void);
 

  int slapd_listenhost2addr(const char *listenhost, PRNetAddr ***addr);

file modified
+28 -21 
@@ -734,7 +734,6 @@ 
 

       * etc the backends need to start
 

       */
 

  

- 

      /* Important: up 'till here we could be running as root (on unix).
 

       * we believe that we've not created any files before here, otherwise
 

       * they'd be owned by root, which is bad. We're about to change identity
 
@@ -891,6 +890,34 @@ 
 

      }
 

      }
 

  

+     if (config_get_disk_monitoring()) {

Any reason you moved this block few lines above where it was ?

 
+         char **dirs = NULL;
 

+         char *dirstr = NULL;
 

+         uint64_t disk_space = 0;
 

+         int64_t threshold = 0;
 

+         uint64_t halfway = 0;
 

+         threshold = config_get_disk_threshold();
 

+         halfway = threshold / 2;
 

+         disk_mon_get_dirs(&dirs);
 

+         dirstr = disk_mon_check_diskspace(dirs, threshold, &disk_space);
 

+         if (dirstr != NULL && disk_space < halfway) {
 

+             slapi_log_err(SLAPI_LOG_EMERG, "main",
 

+                           "Disk Monitoring is enabled and disk space on (%s) is too far below the threshold(%" PRIu64 " bytes). Exiting now.\n",
 

+                           dirstr, threshold);
 

+             slapi_ch_array_free(dirs);
 

+             /*
 

+              * We should free the structs we allocated for sockets and addresses
 

+              * as they would be freed at the slapd_daemon but it was not initiated
 

+              * at that point of start-up.
 

+              */
 

+             slapd_sockets_ports_free(&ports_info);
 

+             return_value = 1;
 

+             goto cleanup;
 

+         }
 

+         slapi_ch_array_free(dirs);
 

+         dirs = NULL;
 

+     }
 

+ 

      /* initialize the normalized DN cache */
 

      if (ndn_cache_init() != 0) {
 

          slapi_log_err(SLAPI_LOG_EMERG, "main", "Unable to create ndn cache\n");
 
@@ -940,26 +967,6 @@ 
 

          slapi_ch_free((void **)&versionstring);
 

      }
 

  

-     if (config_get_disk_monitoring()) {
 

-         char **dirs = NULL;
 

-         char *dirstr = NULL;
 

-         uint64_t disk_space = 0;
 

-         int64_t threshold = 0;
 

-         uint64_t halfway = 0;
 

-         threshold = config_get_disk_threshold();
 

-         halfway = threshold / 2;
 

-         disk_mon_get_dirs(&dirs);
 

-         dirstr = disk_mon_check_diskspace(dirs, threshold, &disk_space);
 

-         if (dirstr != NULL && disk_space < halfway) {
 

-             slapi_log_err(SLAPI_LOG_EMERG, "main",
 

-                           "Disk Monitoring is enabled and disk space on (%s) is too far below the threshold(%" PRIu64 " bytes). Exiting now.\n",
 

-                           dirstr, threshold);
 

-             return_value = 1;
 

-             goto cleanup;
 

-         }
 

-         slapi_ch_array_free(dirs);
 

-         dirs = NULL;
 

-     }
 

      /* log the max fd limit as it is typically set in env/systemd */
 

      slapi_log_err(SLAPI_LOG_INFO, "main",
 

              "Setting the maximum file descriptor limit to: %ld\n",

Description: Fix the rest of the leaks in disk monitoring
which are present when we shutdown while being below half
of the threshold (at the start-up in main.c).

Free directories, sockets and ports before going to cleanup.

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

Reviewed by: ?

Looks reasonable to me. But I'd prefer someone else to have a look as well.

Any reason you moved this block few lines above where it was ?

Any reason you moved this block few lines above where it was ?

I just wanted to do it a bit earlier - before global_backend_lock_init() and write_start_pid_file() calls.
If you think that it is too much of a precaution - I can revert the part.

No I think it is a good idea. I was just curious. ACK

rebased onto 327147c
3 years ago

Pull-Request has been merged by spichugi
3 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/4201

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 3
+41 -34
file changed
ldap/servers/slapd/daemon.c
+1 -0
file changed
ldap/servers/slapd/fe.h
+28 -21
file changed
ldap/servers/slapd/main.c
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.