[lxc-devel] [PATCH] Protect global variables in log module via mutex
S.Çağlar Onur
caglar at 10ur.org
Sat Nov 9 00:24:17 UTC 2013
Log module contains multiple global variables so protect them introducing a new mutex and serialize accessing log functions.
Also gather all locking related code into src/lxc/lxclock.c
Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>
---
src/lxc/log.c | 16 ++++++++++
src/lxc/log.h | 4 +++
src/lxc/lxclock.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++---------
src/lxc/lxclock.h | 5 ++++
src/lxc/utils.c | 57 +-----------------------------------
5 files changed, 100 insertions(+), 69 deletions(-)
diff --git a/src/lxc/log.c b/src/lxc/log.c
index d6ce361..8a5c511 100644
--- a/src/lxc/log.c
+++ b/src/lxc/log.c
@@ -265,11 +265,15 @@ static int __lxc_log_set_file(const char *fname, int create_dirs)
return -1;
}
+ log_lock();
lxc_log_fd = log_open(fname);
+ log_unlock();
if (lxc_log_fd == -1)
return -1;
+ log_lock();
log_fname = strdup(fname);
+ log_unlock();
return 0;
}
@@ -306,15 +310,19 @@ extern int lxc_log_init(const char *name, const char *file,
return -1;
}
+ log_lock();
lxc_loglevel_specified = 1;
lxc_priority = lxc_log_priority_to_int(priority);
+ log_unlock();
}
+ log_lock();
lxc_log_category_lxc.priority = lxc_priority;
lxc_log_category_lxc.appender = &log_appender_logfile;
if (!quiet)
lxc_log_category_lxc.appender->next = &log_appender_stderr;
+ log_unlock();
if (prefix)
lxc_log_set_prefix(prefix);
@@ -322,7 +330,9 @@ extern int lxc_log_init(const char *name, const char *file,
if (file) {
if (strcmp(file, "none") == 0)
return 0;
+ log_lock();
lxc_logfile_specified = 1;
+ log_unlock();
ret = __lxc_log_set_file(file, 1);
} else {
ret = -1;
@@ -368,8 +378,10 @@ extern int lxc_log_set_level(int level)
ERROR("invalid log priority %d", level);
return -1;
}
+ log_lock();
lxc_loglevel_specified = 1;
lxc_log_category_lxc.priority = level;
+ log_unlock();
return 0;
}
@@ -397,7 +409,9 @@ extern int lxc_log_set_file(const char *fname)
{
if (lxc_logfile_specified)
return 0;
+ log_lock();
lxc_logfile_specified = 1;
+ log_unlock();
return __lxc_log_set_file(fname, 0);
}
@@ -408,8 +422,10 @@ extern const char *lxc_log_get_file(void)
extern void lxc_log_set_prefix(const char *prefix)
{
+ log_lock();
strncpy(log_prefix, prefix, sizeof(log_prefix));
log_prefix[sizeof(log_prefix) - 1] = 0;
+ log_unlock();
}
extern const char *lxc_log_get_prefix(void)
diff --git a/src/lxc/log.h b/src/lxc/log.h
index d3c40fb..59e8dd6 100644
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -30,6 +30,8 @@
#include <string.h>
#include <stdbool.h>
+#include "lxclock.h"
+
#ifndef O_CLOEXEC
#define O_CLOEXEC 02000000
#endif
@@ -183,6 +185,7 @@ static inline void LXC_##PRIORITY(struct lxc_log_locinfo *, \
static inline void LXC_##PRIORITY(struct lxc_log_locinfo* locinfo, \
const char* format, ...) \
{ \
+ log_lock(); \
if (lxc_log_priority_is_enabled(acategory, \
LXC_LOG_PRIORITY_##PRIORITY)) { \
struct lxc_log_event evt = { \
@@ -200,6 +203,7 @@ static inline void LXC_##PRIORITY(struct lxc_log_locinfo* locinfo, \
__lxc_log(acategory, &evt); \
va_end(va_ref); \
} \
+ log_unlock(); \
}
/*
diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
index 3857ff0..a255acb 100644
--- a/src/lxc/lxclock.c
+++ b/src/lxc/lxclock.c
@@ -31,6 +31,10 @@
#include <lxc/log.h>
#include <lxc/lxccontainer.h>
+#ifdef MUTEX_DEBUGGING
+#include <execinfo.h>
+#endif
+
#define OFLAG (O_CREAT | O_RDWR)
#define SEMMODE 0660
#define SEMVALUE 1
@@ -40,10 +44,57 @@ lxc_log_define(lxc_lock, lxc);
#ifdef MUTEX_DEBUGGING
pthread_mutex_t thread_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+pthread_mutex_t log_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+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
pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER;
+pthread_mutex_t log_mutex = PTHREAD_MUTEX_INITIALIZER;
+pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+inline void dump_stacktrace(void) {;}
#endif
+void lock_mutex(pthread_mutex_t *l)
+{
+ int ret;
+
+ if ((ret = pthread_mutex_lock(l)) != 0) {
+ fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret, strerror(ret));
+ dump_stacktrace();
+ exit(1);
+ }
+}
+
+void unlock_mutex(pthread_mutex_t *l)
+{
+ int ret;
+
+ if ((ret = pthread_mutex_unlock(l)) != 0) {
+ fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret, strerror(ret));
+ dump_stacktrace();
+ exit(1);
+ }
+}
+
static char *lxclock_name(const char *p, const char *n)
{
int ret;
@@ -267,24 +318,34 @@ void lxc_putlock(struct lxc_lock *l)
void process_lock(void)
{
- int ret;
-
- if ((ret = pthread_mutex_lock(&thread_mutex)) != 0) {
- ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret));
- dump_stacktrace();
- exit(1);
- }
+ lock_mutex(&thread_mutex);
}
void process_unlock(void)
{
- int ret;
+ unlock_mutex(&thread_mutex);
+}
- if ((ret = pthread_mutex_unlock(&thread_mutex)) != 0) {
- ERROR("pthread_mutex_unlock returned:%d %s", ret, strerror(ret));
- dump_stacktrace();
- exit(1);
- }
+/* Protects global log variables and serializes the writes */
+void log_lock(void)
+{
+ lock_mutex(&log_mutex);
+}
+
+void log_unlock(void)
+{
+ unlock_mutex(&log_mutex);
+}
+
+/* Protects static const values inside the lxc_global_config_value funtion */
+void static_lock(void)
+{
+ lock_mutex(&static_mutex);
+}
+
+void static_unlock(void)
+{
+ unlock_mutex(&static_mutex);
}
int container_mem_lock(struct lxc_container *c)
diff --git a/src/lxc/lxclock.h b/src/lxc/lxclock.h
index dcdf79d..a9d7bd4 100644
--- a/src/lxc/lxclock.h
+++ b/src/lxc/lxclock.h
@@ -87,6 +87,11 @@ extern void lxc_putlock(struct lxc_lock *l);
extern void process_lock(void);
extern void process_unlock(void);
+extern void log_lock(void);
+extern void log_unlock(void);
+extern void static_lock(void);
+extern void static_unlock(void);
+
struct lxc_container;
extern int container_mem_lock(struct lxc_container *c);
extern void container_mem_unlock(struct lxc_container *c);
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 4bc2c35..3fab9ae 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -39,11 +39,6 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <assert.h>
-#include <pthread.h>
-
-#ifdef MUTEX_DEBUGGING
-#include <execinfo.h>
-#endif
#ifndef HAVE_GETLINE
#ifdef HAVE_FGETLN
@@ -59,57 +54,6 @@
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;
@@ -392,6 +336,7 @@ out:
if (fin)
fclose(fin);
process_unlock();
+
static_lock();
value = values[i];
static_unlock();
--
1.8.3.2
More information about the lxc-devel
mailing list