Commit 052616eb by S.Çağlar Onur Committed by Serge Hallyn

valgrind drd tool shows conflicting stores happening at…

valgrind drd tool shows conflicting stores happening at lxc_global_config_value@src/lxc/utils.c (v2) 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 avatarS.Çağlar Onur <caglar@10ur.org> Signed-off-by: 's avatarSerge Hallyn <serge.hallyn@ubuntu.com>
parent 4de2791f
...@@ -178,6 +178,15 @@ AM_COND_IF([ENABLE_PYTHON], ...@@ -178,6 +178,15 @@ AM_COND_IF([ENABLE_PYTHON],
PKG_CHECK_MODULES([PYTHONDEV], [python3 >= 3.2],[],[AC_MSG_ERROR([You must install python3-dev])]) PKG_CHECK_MODULES([PYTHONDEV], [python3 >= 3.2],[],[AC_MSG_ERROR([You must install python3-dev])])
AC_DEFINE_UNQUOTED([ENABLE_PYTHON], 1, [Python3 is available])]) 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 # Not in older autoconf versions
# AS_VAR_COPY(DEST, SOURCE) # AS_VAR_COPY(DEST, SOURCE)
# ------------------------- # -------------------------
......
...@@ -91,7 +91,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta() ...@@ -91,7 +91,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta()
int saved_errno; int saved_errno;
errno = 0; errno = 0;
cgroup_use = lxc_global_config_value("cgroup.use"); cgroup_use = default_cgroup_use();
if (!cgroup_use && errno != 0) if (!cgroup_use && errno != 0)
return NULL; return NULL;
if (cgroup_use) { if (cgroup_use) {
......
...@@ -18,15 +18,15 @@ ...@@ -18,15 +18,15 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/ */
#include <pthread.h> #define _GNU_SOURCE
#include "lxclock.h" #include "lxclock.h"
#include <malloc.h> #include <malloc.h>
#include <stdio.h> #include <stdio.h>
#include <errno.h> #include <errno.h>
#include <unistd.h> #include <unistd.h>
#include <fcntl.h> #include <fcntl.h>
#define _GNU_SOURCE
#include <stdlib.h> #include <stdlib.h>
#include <pthread.h>
#include <lxc/utils.h> #include <lxc/utils.h>
#include <lxc/log.h> #include <lxc/log.h>
#include <lxc/lxccontainer.h> #include <lxc/lxccontainer.h>
...@@ -38,7 +38,11 @@ ...@@ -38,7 +38,11 @@
lxc_log_define(lxc_lock, lxc); 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; pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER;
#endif
static char *lxclock_name(const char *p, const char *n) static char *lxclock_name(const char *p, const char *n)
{ {
...@@ -267,13 +271,20 @@ void process_lock(void) ...@@ -267,13 +271,20 @@ void process_lock(void)
if ((ret = pthread_mutex_lock(&thread_mutex)) != 0) { if ((ret = pthread_mutex_lock(&thread_mutex)) != 0) {
ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret)); ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret));
dump_stacktrace();
exit(1); exit(1);
} }
} }
void process_unlock(void) 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) int container_mem_lock(struct lxc_container *c)
......
...@@ -695,7 +695,7 @@ int lxc_spawn(struct lxc_handler *handler) ...@@ -695,7 +695,7 @@ int lxc_spawn(struct lxc_handler *handler)
* default value is available * default value is available
*/ */
if (getuid() == 0) if (getuid() == 0)
cgroup_pattern = lxc_global_config_value("cgroup.pattern"); cgroup_pattern = default_cgroup_pattern();
if (!cgroup_pattern) if (!cgroup_pattern)
cgroup_pattern = "%n"; cgroup_pattern = "%n";
......
...@@ -21,7 +21,8 @@ ...@@ -21,7 +21,8 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/ */
#define _GNU_SOURCE #include "config.h"
#include <errno.h> #include <errno.h>
#include <unistd.h> #include <unistd.h>
#include <stdlib.h> #include <stdlib.h>
...@@ -38,6 +39,8 @@ ...@@ -38,6 +39,8 @@
#include <sys/types.h> #include <sys/types.h>
#include <sys/wait.h> #include <sys/wait.h>
#include <assert.h> #include <assert.h>
#include <pthread.h>
#include <execinfo.h>
#ifndef HAVE_GETLINE #ifndef HAVE_GETLINE
#ifdef HAVE_FGETLN #ifdef HAVE_FGETLN
...@@ -49,8 +52,61 @@ ...@@ -49,8 +52,61 @@
#include "log.h" #include "log.h"
#include "lxclock.h" #include "lxclock.h"
#define MAX_STACKDEPTH 25
lxc_log_define(lxc_utils, lxc); 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) static int _recursive_rmdir_onedev(char *dirname, dev_t pdev)
{ {
struct dirent dirent, *direntp; struct dirent dirent, *direntp;
...@@ -252,8 +308,10 @@ const char *lxc_global_config_value(const char *option_name) ...@@ -252,8 +308,10 @@ const char *lxc_global_config_value(const char *option_name)
{ "cgroup.use", NULL }, { "cgroup.use", NULL },
{ NULL, NULL }, { NULL, NULL },
}; };
/* Protected by a mutex to eliminate conflicting load and store operations */
static const char *values[sizeof(options) / sizeof(options[0])] = { 0 }; static const char *values[sizeof(options) / sizeof(options[0])] = { 0 };
const char *(*ptr)[2]; const char *(*ptr)[2];
const char *value;
size_t i; size_t i;
char buf[1024], *p, *p2; char buf[1024], *p, *p2;
FILE *fin = NULL; FILE *fin = NULL;
...@@ -266,8 +324,14 @@ const char *lxc_global_config_value(const char *option_name) ...@@ -266,8 +324,14 @@ const char *lxc_global_config_value(const char *option_name)
errno = EINVAL; errno = EINVAL;
return NULL; return NULL;
} }
if (values[i])
return values[i]; static_lock();
if (values[i]) {
value = values[i];
static_unlock();
return value;
}
static_unlock();
process_lock(); process_lock();
fin = fopen_cloexec(LXC_GLOBAL_CONF, "r"); fin = fopen_cloexec(LXC_GLOBAL_CONF, "r");
...@@ -304,24 +368,31 @@ const char *lxc_global_config_value(const char *option_name) ...@@ -304,24 +368,31 @@ const char *lxc_global_config_value(const char *option_name)
while (*p && (*p == ' ' || *p == '\t')) p++; while (*p && (*p == ' ' || *p == '\t')) p++;
if (!*p) if (!*p)
continue; continue;
static_lock();
values[i] = copy_global_config_value(p); values[i] = copy_global_config_value(p);
static_unlock();
goto out; goto out;
} }
} }
/* could not find value, use default */ /* could not find value, use default */
static_lock();
values[i] = (*ptr)[1]; values[i] = (*ptr)[1];
/* special case: if default value is NULL, /* special case: if default value is NULL,
* and there is no config, don't view that * and there is no config, don't view that
* as an error... */ * as an error... */
if (!values[i]) if (!values[i])
errno = 0; errno = 0;
static_unlock();
out: out:
process_lock(); process_lock();
if (fin) if (fin)
fclose(fin); fclose(fin);
process_unlock(); process_unlock();
return values[i]; static_lock();
value = values[i];
static_unlock();
return value;
} }
const char *default_lvm_vg(void) const char *default_lvm_vg(void)
...@@ -338,11 +409,22 @@ const char *default_zfs_root(void) ...@@ -338,11 +409,22 @@ const char *default_zfs_root(void)
{ {
return lxc_global_config_value("zfsroot"); return lxc_global_config_value("zfsroot");
} }
const char *default_lxc_path(void) const char *default_lxc_path(void)
{ {
return lxc_global_config_value("lxcpath"); 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 *get_rundir()
{ {
const char *rundir; const char *rundir;
......
...@@ -49,7 +49,8 @@ extern const char *default_lxc_path(void); ...@@ -49,7 +49,8 @@ extern const char *default_lxc_path(void);
extern const char *default_zfs_root(void); extern const char *default_zfs_root(void);
extern const char *default_lvm_vg(void); extern const char *default_lvm_vg(void);
extern const char *default_lvm_thin_pool(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 */ /* Define getline() if missing from the C library */
#ifndef HAVE_GETLINE #ifndef HAVE_GETLINE
#ifdef HAVE_FGETLN #ifdef HAVE_FGETLN
...@@ -239,4 +240,6 @@ extern size_t lxc_array_len(void **array); ...@@ -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_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 **lxc_append_null_to_array(void **array, size_t count);
extern void dump_stacktrace(void);
#endif #endif
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment