debug/sink: add xlator to aid in resource leak debugging

This new xlator does not allocate any resources on init(). This makes it
a good option to use for debugging xlator releated resources leaks on
fini().

By putting the sink xlator as single xlator in a .vol file, and loading
it through gfapi, we can investigate the resource leaks that are
happening through gfapi (and the Gluster core). By extending the .vol
file with additional xlators, it is possible to analyze resource leaks
of single xlators.

Change-Id: Idb5faa861b623dd5b2a988b181e669b0d52c2a0e
BUG: 1425623
Fixes: #176
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: https://review.gluster.org/16806
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>

10 files changed, 313 insertions, 1 deletions

diff --git a/configure.ac b/configure.ac
index 86a8ec17bf7..af3644f4350 100644
--- a/configure.ac
+++ b/configure.ac
@@ -106,6 +106,8 @@ AC_CONFIG_FILES([Makefile
                 xlators/performance/nl-cache/Makefile
                 xlators/performance/nl-cache/src/Makefile
                 xlators/debug/Makefile
+                xlators/debug/sink/Makefile
+                xlators/debug/sink/src/Makefile
                 xlators/debug/trace/Makefile
                 xlators/debug/trace/src/Makefile
                 xlators/debug/error-gen/Makefile
diff --git a/doc/developer-guide/Developers-Index.md b/doc/developer-guide/Developers-Index.md
index 9bcbcdc4cbe..4c6346e83d4 100644
--- a/doc/developer-guide/Developers-Index.md
+++ b/doc/developer-guide/Developers-Index.md
@@ -67,6 +67,7 @@ Testing/Debugging
     Framework](./Using Gluster Test Framework.md) - Step by
     step instructions for running the Gluster Test Framework
 -   [Coredump Analysis](./coredump-analysis.md) - Steps to analize coredumps generated by regression machines.
+-   [Identifying Resource Leaks](./identifying-resource-leaks.md)
 
 Release Process
 ---------------
diff --git a/doc/developer-guide/identifying-resource-leaks.md b/doc/developer-guide/identifying-resource-leaks.md
new file mode 100644
index 00000000000..851fc4424bc
--- /dev/null
+++ b/doc/developer-guide/identifying-resource-leaks.md
@@ -0,0 +1,176 @@
+# Identifying Resource Leaks
+
+Like most other pieces of software, GlusterFS is not perfect in how it manages
+its resources like memory, threads and the like. Gluster developers try hard to
+prevent leaking resources but releasing and unallocating the used structures.
+Unfortunately every now and then some resource leaks are unintentionally added.
+
+This document tries to explain a few helpful tricks to identify resource leaks
+so that they can be addressed.
+
+
+## Debug Builds
+
+There are certain techniques used in GlusterFS that make it difficult to use
+tools like Valgrind for memory leak detection. There are some build options
+that make it more practical to use Valgrind and other tools. When running
+Valgrind, it is important to have GlusterFS builds that contain the
+debuginfo/symbols. Some distributions (try to) strip the debuginfo to get
+smaller executables. Fedora and RHEL based distributions have sub-packages
+called ...-debuginfo that need to be installed for symbol resolving.
+
+
+### Memory Pools
+
+By using memory pools, there are no allocation/freeing of single structures
+needed. This improves performance, but also makes it impossible to track the
+allocation and freeing of srtuctures.
+
+It is possible to disable the use of memory pools, and use standard `malloc()`
+and `free()` functions provided by the C library. Valgrind is then able to
+track the allocated areas and verify if they have been free'd. In order to
+disable memory pools, the Gluster sources needs to be configured with the
+`--enable-debug` option:
+
+```shell
+./configure --enable-debug
+```
+
+When building RPMs, the `.spec` handles the `--with=debug` option too:
+
+```shell
+make dist
+rpmbuild -ta --with=debug glusterfs-....tar.gz
+```
+
+### Dynamically Loaded xlators
+
+Valgrind tracks the call chain of functions that do memory allocations. The
+addresses of the functions are stored and before Valgrind exits the addresses
+are resolved into human readable function names and offsets (line numbers in
+source files). Because Gluster loads xlators dynamically, and unloads then
+before exiting, Valgrind is not able to resolve the function addresses into
+symbols anymore. Whenever this happend, Valgrind shows `???` in the output,
+like
+
+```
+  ==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)
+  ...
+```
+
+These `???` can be prevented by not calling `dlclose()` for unloading the
+xlator. This will cause a small leak of the handle that was returned with
+`dlopen()`, but for improved debugging this can be acceptible. For this and
+other Valgrind features, a `--enable-valgrind` option is available to
+`./configure`. When GlusterFS is built with this option, Valgrind will be able
+to resolve the symbol names of the functions that do memory allocations inside
+xlators.
+
+```shell
+./configure --enable-valgrind
+```
+
+When building RPMs, the `.spec` handles the `--with=valgrind` option too:
+
+```shell
+make dist
+rpmbuild -ta --with=valgrind glusterfs-....tar.gz
+```
+
+## Running Valgrind against a single xlator
+
+Debugging a single xlator is not trivial. But there are some tools to make it
+easier. The `sink` xlator does not do any memory allocations itself, but
+contains just enough functionality to mount a volume with only the `sink`
+xlator. There is a little gfapi application under `tests/basic/gfapi/` in the
+GlusterFS sources that can be used to run only gfapi and the core GlusterFS
+infrastructure with the `sink` xlator. By extending the `.vol` file to load
+more xlators, each xlator can be debugged pretty much separately (as long as
+the xlators have no dependencies on each other). A basic Valgrind run with the
+suitable configure options looks like this:
+
+```shell
+./autogen.sh
+./configure --enable-debug --enable-valgrind
+make && make install
+cd tests/basic/gfapi/
+make gfapi-load-volfile
+valgrind ./gfapi-load-volfile sink.vol
+```
+
+Combined with other very useful options to Valgrind, the following execution
+shows many more useful details:
+
+```shell
+valgrind \
+        --fullpath-after= --leak-check=full --show-leak-kinds=all \
+        ./gfapi-load-volfile sink.vol
+```
+
+Note that the `--fullpath-after=` option is left empty, this makes Valgrind
+print the full path and filename that contains the functions:
+
+```
+==2450== 80 bytes in 1 blocks are definitely lost in loss record 8 of 60
+==2450==    at 0x4C29975: calloc (/builddir/build/BUILD/valgrind-3.11.0/coregrind/m_replacemalloc/vg_replace_malloc.c:711)
+==2450==    by 0x52C6F73: __gf_calloc (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/mem-pool.c:117)
+==2450==    by 0x12F10CDA: init (/usr/src/debug/glusterfs-3.11dev/xlators/meta/src/meta.c:231)
+==2450==    by 0x528EFD5: __xlator_init (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/xlator.c:472)
+==2450==    by 0x528F105: xlator_init (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/xlator.c:498)
+==2450==    by 0x52D9D8B: glusterfs_graph_init (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/graph.c:321)
+...
+```
+
+In the above example, the `init` function in `xlators/meta/src/meta.c` does a
+memory allocation on line 231. This memory is never free'd again, and hence
+Valgrind logs this call stack. When looking in the code, it seems that the
+allocation of `priv` is assigned to the `this->private` member of the
+`xlator_t` structure. Because the allocation is done in `init()`, free'ing is
+expected to happen in `fini()`. Both functions are shown below, with the
+inclusion of the empty `fini()`:
+
+
+```
+226 int
+227 init (xlator_t *this)
+228 {
+229         meta_priv_t *priv = NULL;
+230 
+231         priv = GF_CALLOC (sizeof(*priv), 1, gf_meta_mt_priv_t);
+232         if (!priv)
+233                 return -1;
+234 
+235         GF_OPTION_INIT ("meta-dir-name", priv->meta_dir_name, str, out);
+236 
+237         this->private = priv;
+238 out:
+239         return 0;
+240 }
+241 
+242 
+243 int
+244 fini (xlator_t *this)
+245 {
+246         return 0;
+247 }
+```
+
+In this case, the resource leak can be addressed by adding a single line to the
+`fini()` function:
+
+```
+243 int
+244 fini (xlator_t *this)
+245 {
+246         GF_FREE (this->private);
+247         return 0;
+248 }
+```
+
+Running the same Valgrind command and comparing the output will show that the
+memory leak in `xlators/meta/src/meta.c:init` is not reported anymore.
diff --git a/glusterfs.spec.in b/glusterfs.spec.in
index 63bbd1eea73..89cc284fcb3 100644
--- a/glusterfs.spec.in
+++ b/glusterfs.spec.in
@@ -955,6 +955,7 @@ exit 0
 %dir %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug
 %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/error-gen.so
 %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/io-stats.so
+%{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/sink.so
 %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/trace.so
 %if ( ! ( 0%{?rhel} && 0%{?rhel} < 6 ) )
 # RHEL-5 based distributions have a too old openssl
diff --git a/tests/basic/gfapi/sink.t b/tests/basic/gfapi/sink.t
new file mode 100644
index 00000000000..53af2ecf62d
--- /dev/null
+++ b/tests/basic/gfapi/sink.t
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup
+
+TEST build_tester $(dirname ${0})/gfapi-load-volfile.c -lgfapi
+TEST ./$(dirname ${0})/gfapi-load-volfile $(dirname $0)/sink.vol
+
+cleanup_tester $(dirname ${0})/gfapi-load-volfile
+
+cleanup
diff --git a/tests/basic/gfapi/sink.vol b/tests/basic/gfapi/sink.vol
new file mode 100644
index 00000000000..d1c92261448
--- /dev/null
+++ b/tests/basic/gfapi/sink.vol
@@ -0,0 +1,24 @@
+#
+# The sink xlator does not do any memory allocations. It only passes the FOPs
+# through to the next xlator.
+#
+# For testing, there is no next xlator needed, we are only interested in the
+# resource usage of the Gluster core when gfapi is used.
+#
+# Note: The sink xlator does not handle any calls. Mounting is possible, but
+# any I/O needs additional functionality in the sink xlator.
+#
+volume sink
+    type debug/sink
+    # an option is required, otherwise the graph parsing fails
+    option an-option-is-required yes
+end-volume
+
+#
+# It is possible to test the resource usage of other xlators by adding them in
+# the graph before the "sink".
+#
+#volume mdcache-sink
+#    type performance/md-cache
+#    subvolumes sink
+#end-volume
diff --git a/xlators/debug/Makefile.am b/xlators/debug/Makefile.am
index b655554efec..6e476152ddc 100644
--- a/xlators/debug/Makefile.am
+++ b/xlators/debug/Makefile.am
@@ -1,3 +1,3 @@
-SUBDIRS = trace error-gen io-stats
+SUBDIRS = error-gen io-stats sink trace
 
 CLEANFILES = 
diff --git a/xlators/debug/sink/Makefile.am b/xlators/debug/sink/Makefile.am
new file mode 100644
index 00000000000..f2689244371
--- /dev/null
+++ b/xlators/debug/sink/Makefile.am
@@ -0,0 +1,2 @@
+SUBDIRS = src
+
diff --git a/xlators/debug/sink/src/Makefile.am b/xlators/debug/sink/src/Makefile.am
new file mode 100644
index 00000000000..f952c2ce6bc
--- /dev/null
+++ b/xlators/debug/sink/src/Makefile.am
@@ -0,0 +1,14 @@
+xlator_LTLIBRARIES = sink.la
+xlatordir = $(libdir)/glusterfs/$(PACKAGE_VERSION)/xlator/debug
+
+AM_CPPFLAGS = $(GF_CPPFLAGS) -I$(top_srcdir)/libglusterfs/src \
+        -I$(top_builddir)/rpc/xdr/src
+AM_CFLAGS = -Wall $(GF_CFLAGS)
+
+sink_la_LDFLAGS = -module $(GF_XLATOR_DEFAULT_LDFLAGS)
+
+sink_la_SOURCES = sink.c
+sink_la_LIBADD = $(top_builddir)/libglusterfs/src/libglusterfs.la
+
+CLEANFILES =
+
diff --git a/xlators/debug/sink/src/sink.c b/xlators/debug/sink/src/sink.c
new file mode 100644
index 00000000000..dfd5685a969
--- /dev/null
+++ b/xlators/debug/sink/src/sink.c
@@ -0,0 +1,79 @@
+/*
+   Copyright (c) 2017 Red Hat, Inc. <http://www.redhat.com>
+   This file is part of GlusterFS.
+
+   This file is licensed to you under your choice of the GNU Lesser
+   General Public License, version 3 or any later version (LGPLv3 or
+   later), or the GNU General Public License, version 2 (GPLv2), in all
+   cases as published by the Free Software Foundation.
+*/
+
+#include "xlator.h"
+#include "defaults.h"
+
+int32_t
+init (xlator_t *this)
+{
+        return 0;
+}
+
+void
+fini (xlator_t *this)
+{
+        return;
+}
+
+/*
+ * notify - when parent sends PARENT_UP, send CHILD_UP event from here
+ */
+int32_t
+notify (xlator_t *this, int32_t event, void *data, ...)
+{
+        switch (event) {
+        case GF_EVENT_PARENT_UP:
+                /* Tell the parent that this xlator is up */
+                default_notify (this, GF_EVENT_CHILD_UP, data);
+                break;
+        case GF_EVENT_PARENT_DOWN:
+                /* Tell the parent that this xlator is down */
+                default_notify (this, GF_EVENT_CHILD_DOWN, data);
+                break;
+        default:
+                break;
+        }
+
+        return 0;
+}
+
+/*
+ * A lookup on "/" is done while mounting or glfs_init() is performed. This
+ * needs to return a valid directory for the root of the mountpoint.
+ *
+ * In case this xlator is used for more advanced debugging, it will need to be
+ * extended to support different LOOKUPs too.
+ */
+static int32_t
+sink_lookup (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
+{
+        struct iatt stbuf = { 0, };
+        struct iatt postparent = { 0, };
+
+        /* the root of the volume always need to be a directory */
+        stbuf.ia_type = IA_IFDIR;
+
+        STACK_UNWIND_STRICT (lookup, frame, 0, 0, loc ? loc->inode : NULL,
+                             &stbuf, xdata, &postparent);
+
+        return 0;
+}
+
+struct xlator_fops fops = {
+        .lookup = sink_lookup,
+};
+
+struct xlator_cbks cbks = {
+};
+
+struct volume_options options[] = {
+        { .key  = {NULL} },
+};