xlator: do not call dlclose() when debugging

Valgrind can not show the symbols if a .so after calling dlclose(). The
unhelpful ??? in the output gets resolved properly with this change:

  ==25170== 344 bytes in 1 blocks are definitely lost in loss record 233 of 324
  ==25170==    at 0x4C29975: calloc (vg_replace_malloc.c:711)
  ==25170==    by 0x52C7C0B: __gf_calloc (mem-pool.c:117)
  ==25170==    by 0x12B0638A: ???
  ==25170==    by 0x528FCE6: __xlator_init (xlator.c:472)
  ==25170==    by 0x528FE16: xlator_init (xlator.c:498)
  ==25170==    by 0x52DA8D6: glusterfs_graph_init (graph.c:321)
  ==25170==    by 0x52DB587: glusterfs_graph_activate (graph.c:695)
  ==25170==    by 0x5046407: glfs_process_volfp (glfs-mgmt.c:79)
  ==25170==    by 0x5043B9E: glfs_volumes_init (glfs.c:281)
  ==25170==    by 0x5044FEC: glfs_init_common (glfs.c:986)
  ==25170==    by 0x50451A7: glfs_init@@GFAPI_3.4.0 (glfs.c:1031)

By not calling dlclose(), the dynamically loaded .so is still available
upon program exit, and Valgrind is able to resolve the symbols. This
will add an additional leak, so dlclose() is called for normal builds,
but skipped when configuring with "./configure --enable-valgrind" or
passing the "run-with-valgrind" xlator option.

URL: http://valgrind.org/docs/manual/faq.html#faq.unhelpful
Change-Id: I2044e21b1b8fcce32ad1a817fdd795218f967731
BUG: 1425623
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: https://review.gluster.org/16809
Smoke: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Samikshan Bairagya <samikshan@gmail.com>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>

12 files changed, 45 insertions, 12 deletions

diff --git a/configure.ac b/configure.ac
index c2d03e7ee6a..4bc5bf1d879 100644
--- a/configure.ac
+++ b/configure.ac
@@ -321,6 +321,15 @@ case $host_os in
     ;;
 esac
 
+# --enable-valgrind prevents calling dlclose(), this leaks memory
+AC_ARG_ENABLE([valgrind],
+              AC_HELP_STRING([--enable-valgrind],
+                             [Enable valgrind for resource leak debugging.]))
+if test "x$enable_valgrind" = "xyes"; then
+        AC_DEFINE(RUN_WITH_VALGRIND, 1, [define if all processes should run under valgrind])
+fi
+
+
 AC_ARG_WITH([previous-options],
         [AS_HELP_STRING([--with-previous-options],
                         [read config.status for configure options])
diff --git a/glusterfs.spec.in b/glusterfs.spec.in
index 6225d12e631..7a50a8e5ec5 100644
--- a/glusterfs.spec.in
+++ b/glusterfs.spec.in
@@ -13,6 +13,10 @@
 # rpmbuild -ta @PACKAGE_NAME@-@PACKAGE_VERSION@.tar.gz --with debug
 %{?_with_debug:%global _with_debug --enable-debug}
 
+# if you wish to compile an rpm to run all processes under valgrind...
+# rpmbuild -ta @PACKAGE_NAME@-@PACKAGE_VERSION@.tar.gz --with valgrind
+%{?_with_valgrind:%global _with_valgrind --enable-valgrind}
+
 # if you wish to compile an rpm with cmocka unit testing...
 # rpmbuild -ta @PACKAGE_NAME@-@PACKAGE_VERSION@.tar.gz --with cmocka
 %{?_with_cmocka:%global _with_cmocka --enable-cmocka}
@@ -550,6 +554,9 @@ Obsoletes:        %{name}-geo-replication = %{version}-%{release}
 Requires:         python-argparse
 %endif
 Requires:         pyxattr
+%if (0%{?_with_valgrind:1})
+Requires:         valgrind
+%endif
 
 %description server
 GlusterFS is a distributed file-system capable of scaling to several
@@ -618,6 +625,7 @@ export CFLAGS
 %configure \
         %{?_with_cmocka} \
         %{?_with_debug} \
+        %{?_with_valgrind} \
         %{?_with_tmpfilesdir} \
         %{?_without_bd} \
         %{?_without_epoll} \
diff --git a/libglusterfs/src/ctx.c b/libglusterfs/src/ctx.c
index b009e6270a2..d3fb39822d1 100644
--- a/libglusterfs/src/ctx.c
+++ b/libglusterfs/src/ctx.c
@@ -37,6 +37,10 @@ glusterfs_ctx_new ()
 
         ctx->log.loglevel = DEFAULT_LOG_LEVEL;
 
+#ifdef RUN_WITH_VALGRIND
+        ctx->cmd_args.valgrind = _gf_true;
+#endif
+
         /* lock is never destroyed! */
 	ret = LOCK_INIT (&ctx->lock);
 	if (ret) {
diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index 4f1f27b5857..ce0dde22d4a 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -401,6 +401,11 @@ struct _cmd_args {
 #ifdef GF_LINUX_HOST_OS
         char            *oom_score_adj;
 #endif
+
+        /* Run this process with valgrind? Might want to prevent calling
+         * functions that prevent valgrind from working correctly, like
+         * dlclose(). */
+        int             valgrind;
 };
 typedef struct _cmd_args cmd_args_t;
 
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c
index 408012e7846..b1ed094e0ae 100644
--- a/libglusterfs/src/xlator.c
+++ b/libglusterfs/src/xlator.c
@@ -673,7 +673,7 @@ xlator_members_free (xlator_t *xl)
 
         GF_FREE (xl->name);
         GF_FREE (xl->type);
-        if (xl->dlhandle)
+        if (!(xl->ctx && xl->ctx->cmd_args.valgrind) && xl->dlhandle)
                 dlclose (xl->dlhandle);
         if (xl->options)
                 dict_unref (xl->options);
diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
index 9ff2541c63c..60d39cbd774 100644
--- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c
+++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
@@ -183,6 +183,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr,
                               size_t len, int cmd, defrag_cbk_fn_t cbk,
                               glusterd_op_t op)
 {
+        xlator_t               *this = NULL;
         int                    ret = -1;
         glusterd_defrag_info_t *defrag =  NULL;
         runner_t               runner = {0,};
@@ -195,7 +196,11 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr,
         char                   valgrind_logfile[PATH_MAX] = {0,};
         char                   *volfileserver = NULL;
 
-        priv    = THIS->private;
+        this    = THIS;
+        GF_VALIDATE_OR_GOTO ("glusterd", this, out);
+
+        priv    = this->private;
+        GF_VALIDATE_OR_GOTO ("glusterd", priv, out);
 
         GF_ASSERT (volinfo);
         GF_ASSERT (op_errstr);
@@ -228,7 +233,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr,
         GLUSTERD_GET_DEFRAG_DIR (defrag_path, volinfo, priv);
         ret = mkdir_p (defrag_path, 0777, _gf_true);
         if (ret) {
-                gf_msg (THIS->name, GF_LOG_ERROR, errno,
+                gf_msg (this->name, GF_LOG_ERROR, errno,
                         GD_MSG_CREATE_DIR_FAILED, "Failed to create "
                         "directory %s", defrag_path);
                 goto out;
@@ -241,7 +246,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr,
                     (cmd == GF_DEFRAG_CMD_START_TIER ? "tier":"rebalance"));
         runinit (&runner);
 
-        if (priv->valgrind) {
+        if (this->ctx->cmd_args.valgrind) {
                 snprintf (valgrind_logfile, PATH_MAX,
                           "%s/valgrind-%s-rebalance.log",
                           DEFAULT_LOG_FILE_DIRECTORY,
@@ -255,7 +260,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr,
 
         snprintf (volname, sizeof(volname), "rebalance/%s", volinfo->volname);
 
-        if (dict_get_str (THIS->options, "transport.socket.bind-address",
+        if (dict_get_str (this->options, "transport.socket.bind-address",
                           &volfileserver) == 0) {
                /*In the case of running multiple glusterds on a single machine,
                 *we should ensure that log file and unix socket file shouls be
diff --git a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c
index b57542a85a7..59d8fbdfa6a 100644
--- a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c
+++ b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c
@@ -280,7 +280,7 @@ glusterd_snapdsvc_start (glusterd_svc_t *svc, int flags)
         }
         runinit (&runner);
 
-        if (priv->valgrind) {
+        if (this->ctx->cmd_args.valgrind) {
                 snprintf (valgrind_logfile, PATH_MAX, "%s/valgrind-snapd.log",
                           svc->proc.logdir);
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
index d6e57a432cd..4d60c856171 100644
--- a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
+++ b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
@@ -171,7 +171,7 @@ glusterd_svc_start (glusterd_svc_t *svc, int flags, dict_t *cmdline)
 
         runinit (&runner);
 
-        if (priv->valgrind) {
+        if (this->ctx->cmd_args.valgrind) {
                 snprintf (valgrind_logfile, PATH_MAX, "%s/valgrind-%s.log",
                           svc->proc.logfile, svc->name);
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c b/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c
index bfc879a3436..638d4941521 100644
--- a/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c
+++ b/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c
@@ -304,7 +304,7 @@ glusterd_tierdsvc_start (glusterd_svc_t *svc, int flags)
         }
         runinit (&runner);
 
-        if (priv->valgrind) {
+        if (this->ctx->cmd_args.valgrind) {
                 snprintf (valgrind_logfile, PATH_MAX, "%s/valgrind-tierd.log",
                           svc->proc.logdir);
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 89bfa3d7358..2a9407f3bb4 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -1894,7 +1894,7 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t  *volinfo,
 retry:
         runinit (&runner);
 
-        if (priv->valgrind) {
+        if (this->ctx->cmd_args.valgrind) {
                 /* Run bricks with valgrind */
                 if (volinfo->logdir) {
                         snprintf (valgrind_logfile, PATH_MAX,
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index d26e959d3a3..0f6d888b968 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -1424,6 +1424,7 @@ init (xlator_t *this)
         char              *mountbroker_root           = NULL;
         int                i                          = 0;
         int                total_transport            = 0;
+        gf_boolean_t       valgrind                   = _gf_false;
         char              *valgrind_str               = NULL;
         char              *transport_type             = NULL;
         char               var_run_dir[PATH_MAX]      = {0,};
@@ -1781,17 +1782,19 @@ init (xlator_t *this)
          }
 
         /* Set option to run bricks on valgrind if enabled in glusterd.vol */
-        conf->valgrind = _gf_false;
+        this->ctx->cmd_args.valgrind = valgrind;
         ret = dict_get_str (this->options, "run-with-valgrind", &valgrind_str);
         if (ret < 0) {
                 gf_msg_debug (this->name, 0,
                         "cannot get run-with-valgrind value");
         }
         if (valgrind_str) {
-                if (gf_string2boolean (valgrind_str, &(conf->valgrind))) {
+                if (gf_string2boolean (valgrind_str, &valgrind)) {
                         gf_msg (this->name, GF_LOG_WARNING, EINVAL,
                                 GD_MSG_INVALID_ENTRY,
                                 "run-with-valgrind value not a boolean string");
+                } else {
+                        this->ctx->cmd_args.valgrind = valgrind;
                 }
         }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h
index a773a2b6221..55879aa22de 100644
--- a/xlators/mgmt/glusterd/src/glusterd.h
+++ b/xlators/mgmt/glusterd/src/glusterd.h
@@ -171,7 +171,6 @@ typedef struct {
                                                  * transaction ids */
 
         struct cds_list_head       mount_specs;
-        gf_boolean_t               valgrind;
         pthread_t                  brick_thread;
         void                      *hooks_priv;