Conflict occurs between following lines

[...]
269         if (values[i])
270                 return values[i];
[...]

and

[...]
309         /* could not find value, use default */
310         values[i] = (*ptr)[1];
[...]

fix it using a specific lock dedicated to that problem as Serge suggested.

Also introduce a new autoconf parameter (--enable-mutex-debugging) to convert 
mutexes to error reporting type and to provide a stacktrace when locking fails.

Signed-off-by: S.Çağlar Onur <cag...@10ur.org>
---
 configure.ac      |  9 ++++++
 src/lxc/cgroup.c  |  2 +-
 src/lxc/lxclock.c | 17 +++++++++--
 src/lxc/start.c   |  2 +-
 src/lxc/utils.c   | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/lxc/utils.h   |  5 +++-
 6 files changed, 115 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9fedf55..6004b35 100644
--- a/configure.ac
+++ b/configure.ac
@@ -178,6 +178,15 @@ AM_COND_IF([ENABLE_PYTHON],
        PKG_CHECK_MODULES([PYTHONDEV], [python3 >= 3.2],[],[AC_MSG_ERROR([You 
must install python3-dev])])
        AC_DEFINE_UNQUOTED([ENABLE_PYTHON], 1, [Python3 is available])])
 
+# Enable dumping stack traces
+AC_ARG_ENABLE([mutex-debugging],
+       [AC_HELP_STRING([--enable-mutex-debugging], [Makes mutexes to report 
error and provide stack trace])],
+       [enable_mutex_debugging=yes], [enable_mutex_debugging=no])
+AM_CONDITIONAL([MUTEX_DEBUGGING], [test "x$enable_mutex_debugging" = "xyes"])
+
+AM_COND_IF([MUTEX_DEBUGGING],
+       AC_DEFINE_UNQUOTED([MUTEX_DEBUGGING], 1, [Enabling mutex debugging]))
+
 # Not in older autoconf versions
 # AS_VAR_COPY(DEST, SOURCE)
 # -------------------------
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 01ed040..1e1e72a 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -91,7 +91,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta()
        int saved_errno;
 
        errno = 0;
-       cgroup_use = lxc_global_config_value("cgroup.use");
+       cgroup_use = default_cgroup_use();
        if (!cgroup_use && errno != 0)
                return NULL;
        if (cgroup_use) {
diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
index d403bcc..3857ff0 100644
--- a/src/lxc/lxclock.c
+++ b/src/lxc/lxclock.c
@@ -18,15 +18,15 @@
  *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
  */
 
-#include <pthread.h>
+#define _GNU_SOURCE
 #include "lxclock.h"
 #include <malloc.h>
 #include <stdio.h>
 #include <errno.h>
 #include <unistd.h>
 #include <fcntl.h>
-#define _GNU_SOURCE
 #include <stdlib.h>
+#include <pthread.h>
 #include <lxc/utils.h>
 #include <lxc/log.h>
 #include <lxc/lxccontainer.h>
@@ -38,7 +38,11 @@
 
 lxc_log_define(lxc_lock, lxc);
 
+#ifdef MUTEX_DEBUGGING
+pthread_mutex_t thread_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+#else
 pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER;
+#endif
 
 static char *lxclock_name(const char *p, const char *n)
 {
@@ -267,13 +271,20 @@ void process_lock(void)
 
        if ((ret = pthread_mutex_lock(&thread_mutex)) != 0) {
                ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret));
+               dump_stacktrace();
                exit(1);
        }
 }
 
 void process_unlock(void)
 {
-       pthread_mutex_unlock(&thread_mutex);
+       int ret;
+
+       if ((ret = pthread_mutex_unlock(&thread_mutex)) != 0) {
+               ERROR("pthread_mutex_unlock returned:%d %s", ret, 
strerror(ret));
+               dump_stacktrace();
+               exit(1);
+       }
 }
 
 int container_mem_lock(struct lxc_container *c)
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 1cadc09..58e1194 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -695,7 +695,7 @@ int lxc_spawn(struct lxc_handler *handler)
         * default value is available
         */
        if (getuid() == 0)
-               cgroup_pattern = lxc_global_config_value("cgroup.pattern");
+               cgroup_pattern = default_cgroup_pattern();
        if (!cgroup_pattern)
                cgroup_pattern = "%n";
 
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 9e2e326..590482e 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -21,7 +21,8 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#define _GNU_SOURCE
+#include "config.h"
+
 #include <errno.h>
 #include <unistd.h>
 #include <stdlib.h>
@@ -38,6 +39,8 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <assert.h>
+#include <pthread.h>
+#include <execinfo.h>
 
 #ifndef HAVE_GETLINE
 #ifdef HAVE_FGETLN
@@ -49,8 +52,61 @@
 #include "log.h"
 #include "lxclock.h"
 
+#define MAX_STACKDEPTH 25
+
 lxc_log_define(lxc_utils, lxc);
 
+
+#ifdef MUTEX_DEBUGGING
+static pthread_mutex_t static_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+inline void dump_stacktrace(void)
+{
+       void *array[MAX_STACKDEPTH];
+       size_t size;
+       char **strings;
+       size_t i;
+
+       size = backtrace(array, MAX_STACKDEPTH);
+       strings = backtrace_symbols(array, size);
+
+       // Using fprintf here as our logging module is not thread safe
+       fprintf(stderr, "\tObtained %zd stack frames.\n", size);
+
+       for (i = 0; i < size; i++)
+               fprintf(stderr, "\t\t%s\n", strings[i]);
+
+       free (strings);
+}
+#else
+static pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+inline void dump_stacktrace(void) {;}
+#endif
+
+/* Protects static const values inside the lxc_global_config_value funtion */
+static void static_lock(void)
+{
+       int ret;
+
+       if ((ret = pthread_mutex_lock(&static_mutex)) != 0) {
+               ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret));
+               dump_stacktrace();
+               exit(1);
+       }
+}
+
+static void static_unlock(void)
+{
+       int ret;
+
+       if ((ret = pthread_mutex_unlock(&static_mutex)) != 0) {
+               ERROR("pthread_mutex_unlock returned:%d %s", ret, 
strerror(ret));
+               dump_stacktrace();
+               exit(1);
+       }
+}
+
 static int _recursive_rmdir_onedev(char *dirname, dev_t pdev)
 {
        struct dirent dirent, *direntp;
@@ -252,8 +308,10 @@ const char *lxc_global_config_value(const char 
*option_name)
                { "cgroup.use",      NULL            },
                { NULL, NULL },
        };
+       /* Protected by a mutex to eliminate conflicting load and store 
operations */ 
        static const char *values[sizeof(options) / sizeof(options[0])] = { 0 };
        const char *(*ptr)[2];
+       const char *value;
        size_t i;
        char buf[1024], *p, *p2;
        FILE *fin = NULL;
@@ -266,8 +324,14 @@ const char *lxc_global_config_value(const char 
*option_name)
                errno = EINVAL;
                return NULL;
        }
-       if (values[i])
-               return values[i];
+
+       static_lock();
+       if (values[i]) {
+               value = values[i];
+               static_unlock();
+               return value;
+       }
+       static_unlock();
 
        process_lock();
        fin = fopen_cloexec(LXC_GLOBAL_CONF, "r");
@@ -304,24 +368,31 @@ const char *lxc_global_config_value(const char 
*option_name)
                        while (*p && (*p == ' ' || *p == '\t')) p++;
                        if (!*p)
                                continue;
+                       static_lock();
                        values[i] = copy_global_config_value(p);
+                       static_unlock();
                        goto out;
                }
        }
        /* could not find value, use default */
+       static_lock();
        values[i] = (*ptr)[1];
        /* special case: if default value is NULL,
         * and there is no config, don't view that
         * as an error... */
        if (!values[i])
                errno = 0;
+       static_unlock();
 
 out:
        process_lock();
        if (fin)
                fclose(fin);
        process_unlock();
-       return values[i];
+       static_lock();
+       value = values[i];
+       static_unlock();
+       return value;
 }
 
 const char *default_lvm_vg(void)
@@ -338,11 +409,22 @@ const char *default_zfs_root(void)
 {
        return lxc_global_config_value("zfsroot");
 }
+
 const char *default_lxc_path(void)
 {
        return lxc_global_config_value("lxcpath");
 }
 
+const char *default_cgroup_use(void)
+{
+       return lxc_global_config_value("cgroup.use");
+}
+
+const char *default_cgroup_pattern(void)
+{
+       return lxc_global_config_value("cgroup.pattern");
+}
+
 const char *get_rundir()
 {
        const char *rundir;
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index fc46760..9c47560 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -49,7 +49,8 @@ extern const char *default_lxc_path(void);
 extern const char *default_zfs_root(void);
 extern const char *default_lvm_vg(void);
 extern const char *default_lvm_thin_pool(void);
-
+extern const char *default_cgroup_use(void);
+extern const char *default_cgroup_pattern(void);
 /* Define getline() if missing from the C library */
 #ifndef HAVE_GETLINE
 #ifdef HAVE_FGETLN
@@ -239,4 +240,6 @@ extern size_t lxc_array_len(void **array);
 extern void **lxc_dup_array(void **array, lxc_dup_fn element_dup_fn, 
lxc_free_fn element_free_fn);
 
 extern void **lxc_append_null_to_array(void **array, size_t count);
+
+extern void dump_stacktrace(void);
 #endif
-- 
1.8.3.2


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to