logging: Introduce mutex API to manage log_target in multi-thread envs

log_enable_multithread() enables use of locks inside the
implementation. Lock use is disabled by default, this way only
multi-thread processes need to enable it and suffer related
complexity/performance penalties.

Locks are required around osmo_log_target_list and items inside it,
since targets can be used, modified and deleted by different threads
concurrently (for instance, user writing "logging disable" in VTY while
another thread is willing to write into that target).

Multithread apps and libraries aiming at being used in multithread apps
should update their code to use the locks introduced here when
containing code iterating over osmo_log_target_list explictly or
implicitly by obtaining a log_target (eg. osmo_log_vty2tgt()).

Related: OS#4088
Change-Id: Id7711893b34263baacac6caf4d489467053131bb

5 files changed, 199 insertions, 94 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 245eb6d5..5f5f0171 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -4,7 +4,7 @@
 LIBVERSION=14:0:2
 
 AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include
-AM_CFLAGS = -Wall $(TALLOC_CFLAGS)
+AM_CFLAGS = -Wall $(TALLOC_CFLAGS) $(PTHREAD_CFLAGS)
 
 if ENABLE_PSEUDOTALLOC
 AM_CPPFLAGS += -I$(top_srcdir)/src/pseudotalloc
@@ -12,7 +12,7 @@ endif
 
 lib_LTLIBRARIES = libosmocore.la
 
-libosmocore_la_LIBADD = $(BACKTRACE_LIB) $(TALLOC_LIBS) $(LIBRARY_RT)
+libosmocore_la_LIBADD = $(BACKTRACE_LIB) $(TALLOC_LIBS) $(LIBRARY_RT) $(PTHREAD_LIBS)
 libosmocore_la_SOURCES = context.c timer.c timer_gettimeofday.c timer_clockgettime.c \
 			 select.c signal.c msgb.c bits.c \
 			 bitvec.c bitcomp.c counter.c fsm.c \
diff --git a/src/gb/gprs_bssgp_vty.c b/src/gb/gprs_bssgp_vty.c
index 3af6517f..5dab94e7 100644
--- a/src/gb/gprs_bssgp_vty.c
+++ b/src/gb/gprs_bssgp_vty.c
@@ -181,21 +181,27 @@ DEFUN(logging_fltr_bvc,
 	"BVCI of the BVC to be filtered\n"
 	"BSSGP Virtual Connection Identifier (BVCI)\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 	struct bssgp_bvc_ctx *bvc;
 	uint16_t nsei = atoi(argv[0]);
 	uint16_t bvci = atoi(argv[1]);
 
-	if (!tgt)
+	log_tgt_mutex_lock();
+	tgt = osmo_log_vty2tgt(vty);
+	if (!tgt) {
+		log_tgt_mutex_unlock();
 		return CMD_WARNING;
+	}
 
 	bvc = btsctx_by_bvci_nsei(bvci, nsei);
 	if (!bvc) {
 		vty_out(vty, "No BVC by that identifier%s", VTY_NEWLINE);
+		log_tgt_mutex_unlock();
 		return CMD_WARNING;
 	}
 
 	log_set_bvc_filter(tgt, bvc);
+	log_tgt_mutex_unlock();
 	return CMD_SUCCESS;
 }
 
diff --git a/src/gb/gprs_ns_vty.c b/src/gb/gprs_ns_vty.c
index 53c71a9a..4a904368 100644
--- a/src/gb/gprs_ns_vty.c
+++ b/src/gb/gprs_ns_vty.c
@@ -587,12 +587,16 @@ DEFUN(logging_fltr_nsvc,
 	"Identify NS-VC by NSVCI\n"
 	"Numeric identifier\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 	struct gprs_nsvc *nsvc;
 	uint16_t id = atoi(argv[1]);
 
-	if (!tgt)
+	log_tgt_mutex_lock();
+	tgt = osmo_log_vty2tgt(vty);
+	if (!tgt) {
+		log_tgt_mutex_unlock();
 		return CMD_WARNING;
+	}
 
 	if (!strcmp(argv[0], "nsei"))
 		nsvc = gprs_nsvc_by_nsei(vty_nsi, id);
@@ -601,10 +605,12 @@ DEFUN(logging_fltr_nsvc,
 
 	if (!nsvc) {
 		vty_out(vty, "No NS-VC by that identifier%s", VTY_NEWLINE);
+		log_tgt_mutex_unlock();
 		return CMD_WARNING;
 	}
 
 	log_set_nsvc_filter(tgt, nsvc);
+	log_tgt_mutex_unlock();
 	return CMD_SUCCESS;
 }
 
diff --git a/src/logging.c b/src/logging.c
index 1c3544fa..b030f8a6 100644
--- a/src/logging.c
+++ b/src/logging.c
@@ -42,6 +42,7 @@
 #include <time.h>
 #include <sys/time.h>
 #include <errno.h>
+#include <pthread.h>
 
 #include <osmocom/core/talloc.h>
 #include <osmocom/core/utils.h>
@@ -63,6 +64,56 @@ static struct log_context log_context;
 void *tall_log_ctx = NULL;
 LLIST_HEAD(osmo_log_target_list);
 
+#if (!EMBEDDED)
+/*! This mutex must be held while using osmo_log_target_list or any of its
+  log_targets in a multithread program. Prevents race conditions between threads
+  like producing unordered timestamps or VTY deleting a target while another
+  thread is writing to it */
+static pthread_mutex_t osmo_log_tgt_mutex;
+static bool osmo_log_tgt_mutex_on = false;
+
+/*! Enable multithread support (mutex) in libosmocore logging system.
+ * Must be called by processes willing to use logging subsystem from several
+ * threads. Once enabled, it's not possible to disable it again.
+ */
+void log_enable_multithread(void) {
+	if (osmo_log_tgt_mutex_on)
+		return;
+	pthread_mutex_init(&osmo_log_tgt_mutex, NULL);
+	osmo_log_tgt_mutex_on = true;
+}
+
+/*! Acquire the osmo_log_tgt_mutex. Don't use this function directly, always use
+ *  macro log_tgt_mutex_lock() instead.
+ */
+void log_tgt_mutex_lock_impl(void) {
+	/* These lines are useful to debug scenarios where there's only 1 thread
+	   and a double lock appears, for instance during startup and some
+	   unlock() missing somewhere:
+	if (osmo_log_tgt_mutex_on && pthread_mutex_trylock(&osmo_log_tgt_mutex) != 0)
+		osmo_panic("acquiring already locked mutex!\n");
+	return;
+	*/
+
+	if (osmo_log_tgt_mutex_on)
+		pthread_mutex_lock(&osmo_log_tgt_mutex);
+}
+
+/*! Release the osmo_log_tgt_mutex. Don't use this function directly, always use
+ *  macro log_tgt_mutex_unlock() instead.
+ */
+void log_tgt_mutex_unlock_impl(void) {
+	if (osmo_log_tgt_mutex_on)
+		pthread_mutex_unlock(&osmo_log_tgt_mutex);
+}
+
+#else /* if (!EMBEDDED) */
+#pragma message ("logging multithread support disabled in embedded build")
+void log_enable_multithread(void) {}
+void log_tgt_mutex_lock_impl(void) {}
+void log_tgt_mutex_unlock_impl(void) {}
+#endif /* if (!EMBEDDED) */
+
 const struct value_string loglevel_strs[] = {
 	{ LOGL_DEBUG,	"DEBUG" },
 	{ LOGL_INFO,	"INFO" },
@@ -532,6 +583,8 @@ void osmo_vlogp(int subsys, int level, const char *file, int line,
 
 	subsys = map_subsys(subsys);
 
+	log_tgt_mutex_lock();
+
 	llist_for_each_entry(tar, &osmo_log_target_list, entry) {
 		va_list bp;
 
@@ -548,6 +601,8 @@ void osmo_vlogp(int subsys, int level, const char *file, int line,
 			_output(tar, subsys, level, file, line, cont, format, bp);
 		va_end(bp);
 	}
+
+	log_tgt_mutex_unlock();
 }
 
 /*! logging function used by DEBUGP() macro
@@ -870,6 +925,7 @@ struct log_target *log_target_create_file(const char *fname)
  *  \param[in] type Log target type
  *  \param[in] fname File name
  *  \returns Log target (if found), NULL otherwise
+ *  Must be called with mutex osmo_log_tgt_mutex held, see log_tgt_mutex_lock.
  */
 struct log_target *log_target_find(int type, const char *fname)
 {
@@ -942,6 +998,8 @@ int log_targets_reopen(void)
 	struct log_target *tar;
 	int rc = 0;
 
+	log_tgt_mutex_lock();
+
 	llist_for_each_entry(tar, &osmo_log_target_list, entry) {
 		switch (tar->type) {
 		case LOG_TGT_TYPE_FILE:
@@ -953,6 +1011,8 @@ int log_targets_reopen(void)
 		}
 	}
 
+	log_tgt_mutex_unlock();
+
 	return rc;
 }
 
@@ -1015,6 +1075,8 @@ void log_fini(void)
 {
 	struct log_target *tar, *tar2;
 
+	log_tgt_mutex_lock();
+
 	llist_for_each_entry_safe(tar, tar2, &osmo_log_target_list, entry)
 		log_target_destroy(tar);
 
@@ -1022,6 +1084,8 @@ void log_fini(void)
 	osmo_log_info = NULL;
 	talloc_free(tall_log_ctx);
 	tall_log_ctx = NULL;
+
+	log_tgt_mutex_unlock();
 }
 
 /*! Check whether a log entry will be generated.
@@ -1036,15 +1100,19 @@ int log_check_level(int subsys, unsigned int level)
 
 	/* TODO: The following could/should be cached (update on config) */
 
+	log_tgt_mutex_lock();
+
 	llist_for_each_entry(tar, &osmo_log_target_list, entry) {
 		if (!should_log_to_target(tar, subsys, level))
 			continue;
 
 		/* This might get logged (ignoring filters) */
+		log_tgt_mutex_unlock();
 		return 1;
 	}
 
 	/* We are sure, that this will not be logged. */
+	log_tgt_mutex_unlock();
 	return 0;
 }
 
diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c
index d639a8f7..88ee330a 100644
--- a/src/vty/logging_vty.c
+++ b/src/vty/logging_vty.c
@@ -101,6 +101,25 @@ struct log_target *log_target_create_vty(struct vty *vty)
 	return target;
 }
 
+/*! Get tgt with log lock acquired, return and release lock with warning if tgt
+ *  is not found. Lock must be released later with log_tgt_mutex_unlock().
+ */
+#define ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt) \
+	do { \
+		log_tgt_mutex_lock(); \
+		tgt = osmo_log_vty2tgt(vty); \
+		if (!(tgt)) { \
+			log_tgt_mutex_unlock(); \
+			return CMD_WARNING; \
+		} \
+	} while (0)
+
+#define RET_WITH_UNLOCK(ret) \
+	do { \
+		log_tgt_mutex_unlock(); \
+		return (ret); \
+	} while (0)
+
 DEFUN(enable_logging,
       enable_logging_cmd,
       "logging enable",
@@ -118,11 +137,16 @@ DEFUN(enable_logging,
 	conn->dbg = log_target_create_vty(vty);
 	if (!conn->dbg)
 		return CMD_WARNING;
-
+	log_tgt_mutex_lock();
 	log_add_target(conn->dbg);
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
+/*! Get log target associated to VTY console.
+ *  \param[in] vty Log target type
+ *  \returns Log target (if logging enabled), NULL otherwise
+ *  Must be called with mutex osmo_log_tgt_mutex held, see log_tgt_mutex_lock.
+ */
 struct log_target *osmo_log_vty2tgt(struct vty *vty)
 {
 	struct telnet_connection *conn;
@@ -146,13 +170,12 @@ DEFUN(logging_fltr_all,
 	"Only print messages matched by other filters\n"
 	"Bypass filter and print all messages\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_all_filter(tgt, atoi(argv[0]));
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(logging_use_clr,
@@ -162,13 +185,12 @@ DEFUN(logging_use_clr,
       "Don't use color for printing messages\n"
       "Use color for printing messages\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_use_color(tgt, atoi(argv[0]));
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(logging_prnt_timestamp,
@@ -178,13 +200,12 @@ DEFUN(logging_prnt_timestamp,
 	"Don't prefix each log message\n"
 	"Prefix each log message with current timestamp\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_print_timestamp(tgt, atoi(argv[0]));
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(logging_prnt_ext_timestamp,
@@ -195,13 +216,12 @@ DEFUN(logging_prnt_ext_timestamp,
 	"Don't prefix each log message\n"
 	"Prefix each log message with current timestamp with YYYYMMDDhhmmssnnn\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_print_extended_timestamp(tgt, atoi(argv[0]));
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(logging_prnt_cat,
@@ -212,13 +232,11 @@ DEFUN(logging_prnt_cat,
 	"Don't prefix each log message\n"
 	"Prefix each log message with category/subsystem name\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
-
-	if (!tgt)
-		return CMD_WARNING;
+	struct log_target *tgt;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_print_category(tgt, atoi(argv[0]));
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(logging_prnt_cat_hex,
@@ -229,13 +247,12 @@ DEFUN(logging_prnt_cat_hex,
 	"Don't prefix each log message\n"
 	"Prefix each log message with category/subsystem nr in hex ('<000b>')\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_print_category_hex(tgt, atoi(argv[0]));
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(logging_prnt_level,
@@ -246,13 +263,12 @@ DEFUN(logging_prnt_level,
       "Don't prefix each log message\n"
       "Prefix each log message with the log level name\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_print_level(tgt, atoi(argv[0]));
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 static const struct value_string logging_print_file_args[] = {
@@ -273,17 +289,16 @@ DEFUN(logging_prnt_file,
       "Log source file info at the end of a log line. If omitted, log source file info just"
       " before the log text.\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_set_print_filename2(tgt, get_string_value(logging_print_file_args, argv[0]));
 	if (argc > 1)
 		log_set_print_filename_pos(tgt, LOG_FILENAME_POS_LINE_END);
 	else
 		log_set_print_filename_pos(tgt, LOG_FILENAME_POS_HEADER_END);
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 static void add_category_strings(char **cmd_str_p, char **doc_str_p,
@@ -332,27 +347,26 @@ DEFUN(logging_level,
       NULL, /* cmdstr is dynamically set in logging_vty_add_cmds(). */
       NULL) /* same thing for helpstr. */
 {
+	struct log_target *tgt;
 	int category = log_parse_category(argv[0]);
 	int level = log_parse_level(argv[1]);
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	if (level < 0) {
 		vty_out(vty, "Invalid level `%s'%s", argv[1], VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 
 	if (category < 0) {
 		vty_out(vty, "Invalid category `%s'%s", argv[0], VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 
 	tgt->categories[category].enabled = 1;
 	tgt->categories[category].loglevel = level;
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(logging_level_set_all, logging_level_set_all_cmd,
@@ -362,12 +376,11 @@ DEFUN(logging_level_set_all, logging_level_set_all_cmd,
       " to take back these changes -- each category is set to the given level, period.\n"
       LOG_LEVEL_STRS)
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 	int level = log_parse_level(argv[0]);
 	int i;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	for (i = 0; i < osmo_log_info->num_cat; i++) {
 		struct log_category *cat = &tgt->categories[i];
@@ -378,7 +391,7 @@ DEFUN(logging_level_set_all, logging_level_set_all_cmd,
 		cat->enabled = 1;
 		cat->loglevel = level;
 	}
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 /* logging level (<categories>) everything */
@@ -394,23 +407,25 @@ DEFUN(logging_level_force_all, logging_level_force_all_cmd,
       "logging level force-all " LOG_LEVEL_ARGS,
       LOGGING_STR LEVEL_STR FORCE_ALL_STR LOG_LEVEL_STRS)
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 	int level = log_parse_level(argv[0]);
-	if (!tgt)
-		return CMD_WARNING;
+
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
+
 	log_set_log_level(tgt, level);
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(no_logging_level_force_all, no_logging_level_force_all_cmd,
       "no logging level force-all",
       NO_STR LOGGING_STR LEVEL_STR NO_FORCE_ALL_STR)
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
-	if (!tgt)
-		return CMD_WARNING;
+	struct log_target *tgt;
+
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
+
 	log_set_log_level(tgt, 0);
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 /* 'logging level all (debug|...|fatal)' */
@@ -438,13 +453,12 @@ DEFUN(logging_set_category_mask,
       " " OSMO_STRINGIFY(LOGL_FATAL) "=" OSMO_STRINGIFY_VAL(LOGL_FATAL)
       "\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_parse_category_mask(tgt, argv[0]);
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 ALIAS_DEPRECATED(logging_set_category_mask,
@@ -462,17 +476,16 @@ DEFUN(diable_logging,
 	LOGGING_STR
       "Disables logging to this vty\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 	struct telnet_connection *conn = (struct telnet_connection *) vty->priv;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	log_del_target(tgt);
 	talloc_free(tgt);
 	conn->dbg = NULL;
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 static void vty_print_logtarget(struct vty *vty, const struct log_info *info,
@@ -517,14 +530,12 @@ DEFUN(show_logging_vty,
 	SHOW_STR SHOW_LOG_STR
 	"Show current logging configuration for this vty\n")
 {
-	struct log_target *tgt = osmo_log_vty2tgt(vty);
+	struct log_target *tgt;
 
-	if (!tgt)
-		return CMD_WARNING;
+	ACQUIRE_VTY_LOG_TGT_WITH_LOCK(vty, tgt);
 
 	vty_print_logtarget(vty, osmo_log_info, tgt);
-
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(show_alarms,
@@ -535,11 +546,14 @@ DEFUN(show_alarms,
 {
 	int i, num_alarms;
 	struct osmo_strrb *rb;
-	struct log_target *tgt = log_target_find(LOG_TGT_TYPE_STRRB, NULL);
+	struct log_target *tgt;
+
+	log_tgt_mutex_lock();
+	tgt = log_target_find(LOG_TGT_TYPE_STRRB, NULL);
 	if (!tgt) {
 		vty_out(vty, "%% No alarms, run 'log alarms <2-32700>'%s",
 			VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 
 	rb = tgt->tgt_rb.rb;
@@ -550,8 +564,7 @@ DEFUN(show_alarms,
 	for (i = 0; i < num_alarms; i++)
 		vty_out(vty, "%% %s%s", osmo_strrb_get_nth(rb, i),
 			VTY_NEWLINE);
-
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 gDEFUN(cfg_description, cfg_description_cmd,
@@ -625,6 +638,7 @@ static int _cfg_log_syslog(struct vty *vty, int facility)
 {
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	/* First delete the old syslog target, if any */
 	tgt = log_target_find(LOG_TGT_TYPE_SYSLOG, NULL);
 	if (tgt)
@@ -633,14 +647,14 @@ static int _cfg_log_syslog(struct vty *vty, int facility)
 	tgt = log_target_create_syslog(host.app_info->name, 0, facility);
 	if (!tgt) {
 		vty_out(vty, "%% Unable to open syslog%s", VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 	log_add_target(tgt);
 
 	vty->index = tgt;
 	vty->node = CFG_LOG_NODE;
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(cfg_log_syslog_local, cfg_log_syslog_local_cmd,
@@ -700,16 +714,17 @@ DEFUN(cfg_no_log_syslog, cfg_no_log_syslog_cmd,
 {
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_SYSLOG, NULL);
 	if (!tgt) {
 		vty_out(vty, "%% No syslog target found%s",
 			VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 
 	log_target_destroy(tgt);
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 #endif /* HAVE_SYSLOG_H */
 
@@ -721,6 +736,7 @@ DEFUN(cfg_log_gsmtap, cfg_log_gsmtap_cmd,
 	const char *hostname = argc ? argv[0] : "127.0.0.1";
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_GSMTAP, hostname);
 	if (!tgt) {
 		tgt = log_target_create_gsmtap(hostname, GSMTAP_UDP_PORT,
@@ -729,7 +745,7 @@ DEFUN(cfg_log_gsmtap, cfg_log_gsmtap_cmd,
 		if (!tgt) {
 			vty_out(vty, "%% Unable to create GSMTAP log for %s%s",
 				hostname, VTY_NEWLINE);
-			return CMD_WARNING;
+			RET_WITH_UNLOCK(CMD_WARNING);
 		}
 		log_add_target(tgt);
 	}
@@ -737,7 +753,7 @@ DEFUN(cfg_log_gsmtap, cfg_log_gsmtap_cmd,
 	vty->index = tgt;
 	vty->node = CFG_LOG_NODE;
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(cfg_log_stderr, cfg_log_stderr_cmd,
@@ -746,13 +762,14 @@ DEFUN(cfg_log_stderr, cfg_log_stderr_cmd,
 {
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_STDERR, NULL);
 	if (!tgt) {
 		tgt = log_target_create_stderr();
 		if (!tgt) {
 			vty_out(vty, "%% Unable to create stderr log%s",
 				VTY_NEWLINE);
-			return CMD_WARNING;
+			RET_WITH_UNLOCK(CMD_WARNING);
 		}
 		log_add_target(tgt);
 	}
@@ -760,7 +777,7 @@ DEFUN(cfg_log_stderr, cfg_log_stderr_cmd,
 	vty->index = tgt;
 	vty->node = CFG_LOG_NODE;
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(cfg_no_log_stderr, cfg_no_log_stderr_cmd,
@@ -769,15 +786,16 @@ DEFUN(cfg_no_log_stderr, cfg_no_log_stderr_cmd,
 {
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_STDERR, NULL);
 	if (!tgt) {
 		vty_out(vty, "%% No stderr logging active%s", VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 
 	log_target_destroy(tgt);
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(cfg_log_file, cfg_log_file_cmd,
@@ -787,13 +805,14 @@ DEFUN(cfg_log_file, cfg_log_file_cmd,
 	const char *fname = argv[0];
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_FILE, fname);
 	if (!tgt) {
 		tgt = log_target_create_file(fname);
 		if (!tgt) {
 			vty_out(vty, "%% Unable to create file `%s'%s",
 				fname, VTY_NEWLINE);
-			return CMD_WARNING;
+			RET_WITH_UNLOCK(CMD_WARNING);
 		}
 		log_add_target(tgt);
 	}
@@ -801,7 +820,7 @@ DEFUN(cfg_log_file, cfg_log_file_cmd,
 	vty->index = tgt;
 	vty->node = CFG_LOG_NODE;
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 
@@ -812,16 +831,17 @@ DEFUN(cfg_no_log_file, cfg_no_log_file_cmd,
 	const char *fname = argv[0];
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_FILE, fname);
 	if (!tgt) {
 		vty_out(vty, "%% No such log file `%s'%s",
 			fname, VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 
 	log_target_destroy(tgt);
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(cfg_log_alarms, cfg_log_alarms_cmd,
@@ -832,6 +852,8 @@ DEFUN(cfg_log_alarms, cfg_log_alarms_cmd,
 	struct log_target *tgt;
 	unsigned int rbsize = atoi(argv[0]);
 
+
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_STRRB, NULL);
 	if (tgt)
 		log_target_destroy(tgt);
@@ -840,14 +862,14 @@ DEFUN(cfg_log_alarms, cfg_log_alarms_cmd,
 	if (!tgt) {
 		vty_out(vty, "%% Unable to create osmo_strrb (size %u)%s",
 			rbsize, VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 	log_add_target(tgt);
 
 	vty->index = tgt;
 	vty->node = CFG_LOG_NODE;
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 DEFUN(cfg_no_log_alarms, cfg_no_log_alarms_cmd,
@@ -856,15 +878,16 @@ DEFUN(cfg_no_log_alarms, cfg_no_log_alarms_cmd,
 {
 	struct log_target *tgt;
 
+	log_tgt_mutex_lock();
 	tgt = log_target_find(LOG_TGT_TYPE_STRRB, NULL);
 	if (!tgt) {
 		vty_out(vty, "%% No osmo_strrb target found%s", VTY_NEWLINE);
-		return CMD_WARNING;
+		RET_WITH_UNLOCK(CMD_WARNING);
 	}
 
 	log_target_destroy(tgt);
 
-	return CMD_SUCCESS;
+	RET_WITH_UNLOCK(CMD_SUCCESS);
 }
 
 static int config_write_log_single(struct vty *vty, struct log_target *tgt)
@@ -962,11 +985,13 @@ static int config_write_log_single(struct vty *vty, struct log_target *tgt)
 
 static int config_write_log(struct vty *vty)
 {
+	log_tgt_mutex_lock();
 	struct log_target *dbg = vty->index;
 
 	llist_for_each_entry(dbg, &osmo_log_target_list, entry)
 		config_write_log_single(vty, dbg);
 
+	log_tgt_mutex_unlock();
 	return 1;
 }