This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/15560
Change subject: logging: Introduce mutex API to manage log_target in multi-thread envs
......................................................................
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
---
M include/osmocom/core/logging.h
M src/gb/gprs_bssgp_vty.c
M src/gb/gprs_ns_vty.c
M src/logging.c
M src/vty/logging_vty.c
M tests/logging/logging_vty_test.c
6 files changed, 202 insertions(+), 93 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/60/15560/1
diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h
index c2648f3..73df8e9 100644
--- a/include/osmocom/core/logging.h
+++ b/include/osmocom/core/logging.h
@@ -378,4 +378,18 @@
struct log_target *log_target_find(int type, const char *fname);
+void log_enable_multithread(void);
+
+#define LOG_MTX_DEBUG 0
+#if LOG_MTX_DEBUG
+ #include <pthread.h>
+ void _log_tgt_mutex_lock(void);
+ void _log_tgt_mutex_unlock(void);
+ #define log_tgt_mutex_lock() do { fprintf(stderr, "[%lu] %s:%d [%s] lock\n", pthread_self(), __FILE__, __LINE__, __func__); _log_tgt_mutex_lock(); } while (0)
+ #define log_tgt_mutex_unlock() do { fprintf(stderr, "[%lu] %s:%d [%s] unlock\n", pthread_self(), __FILE__, __LINE__, __func__); _log_tgt_mutex_unlock(); } while (0)
+#else
+ void log_tgt_mutex_lock(void);
+ void log_tgt_mutex_unlock(void);
+#endif
+
/*! @} */
diff --git a/src/gb/gprs_bssgp_vty.c b/src/gb/gprs_bssgp_vty.c
index 3af6517..5dab94e 100644
--- a/src/gb/gprs_bssgp_vty.c
+++ b/src/gb/gprs_bssgp_vty.c
@@ -181,21 +181,27 @@
"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 53c71a9..4a90436 100644
--- a/src/gb/gprs_ns_vty.c
+++ b/src/gb/gprs_ns_vty.c
@@ -587,12 +587,16 @@
"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 @@
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 1c3544f..57d6ee4 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,54 @@
void *tall_log_ctx = NULL;
LLIST_HEAD(osmo_log_target_list);
+/*! This mutex must be hold while using osmo_log_target_list or any of its
+ log_targets. Prevents race conditions between threads like producing
+ unordered timestamps or VTY deleting a target while another thread is writing
+ to it */
+pthread_mutex_t osmo_log_tgt_mutex;
+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.
+ */
+#if LOG_MTX_DEBUG
+void _log_tgt_mutex_lock(void) {
+#else
+void log_tgt_mutex_lock(void) {
+#endif
+/* 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.
+ */
+#if LOG_MTX_DEBUG
+void _log_tgt_mutex_unlock(void) {
+#else
+void log_tgt_mutex_unlock(void) {
+#endif
+ if (osmo_log_tgt_mutex_on)
+ pthread_mutex_unlock(&osmo_log_tgt_mutex);
+}
+
const struct value_string loglevel_strs[] = {
{ LOGL_DEBUG, "DEBUG" },
{ LOGL_INFO, "INFO" },
@@ -532,6 +581,8 @@
subsys = map_subsys(subsys);
+ log_tgt_mutex_lock();
+
llist_for_each_entry(tar, &osmo_log_target_list, entry) {
va_list bp;
@@ -548,6 +599,8 @@
_output(tar, subsys, level, file, line, cont, format, bp);
va_end(bp);
}
+
+ log_tgt_mutex_unlock();
}
/*! logging function used by DEBUGP() macro
@@ -866,7 +919,7 @@
}
#endif
-/*! Find a registered log target
+/*! Find a registered log target. Must be called with mutex locked.
* \param[in] type Log target type
* \param[in] fname File name
* \returns Log target (if found), NULL otherwise
@@ -942,6 +995,8 @@
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 +1008,8 @@
}
}
+ log_tgt_mutex_lock();
+
return rc;
}
@@ -1015,6 +1072,8 @@
{
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 +1081,8 @@
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 +1097,19 @@
/* 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 d639a8f..a772fa5 100644
--- a/src/vty/logging_vty.c
+++ b/src/vty/logging_vty.c
@@ -101,6 +101,22 @@
return target;
}
+/* Get tgt with log lock acquired, return and release lock 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,9 +134,9 @@
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);
}
struct log_target *osmo_log_vty2tgt(struct vty *vty)
@@ -146,13 +162,12 @@
"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 +177,12 @@
"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 +192,12 @@
"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 +208,12 @@
"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 +224,11 @@
"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 +239,12 @@
"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 +255,12 @@
"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 +281,16 @@
"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 +339,26 @@
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 +368,11 @@
" 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 +383,7 @@
cat->enabled = 1;
cat->loglevel = level;
}
- return CMD_SUCCESS;
+ RET_WITH_UNLOCK(CMD_SUCCESS);
}
/* logging level (<categories>) everything */
@@ -394,23 +399,25 @@
"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 +445,12 @@
" " 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 +468,16 @@
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 +522,12 @@
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 +538,14 @@
{
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 +556,7 @@
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 +630,7 @@
{
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 +639,14 @@
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 +706,17 @@
{
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 +728,7 @@
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 +737,7 @@
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 +745,7 @@
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 +754,14 @@
{
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 +769,7 @@
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 +778,16 @@
{
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 +797,14 @@
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 +812,7 @@
vty->index = tgt;
vty->node = CFG_LOG_NODE;
- return CMD_SUCCESS;
+ RET_WITH_UNLOCK(CMD_SUCCESS);
}
@@ -812,16 +823,17 @@
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 +844,8 @@
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 +854,14 @@
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 +870,16 @@
{
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 +977,13 @@
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;
}
diff --git a/tests/logging/logging_vty_test.c b/tests/logging/logging_vty_test.c
index 30426f3..e7019f6 100644
--- a/tests/logging/logging_vty_test.c
+++ b/tests/logging/logging_vty_test.c
@@ -241,6 +241,7 @@
vty_init(&vty_info);
osmo_init_logging2(root_ctx, &log_info);
+ log_enable_multithread();
vty_commands_init();
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15560
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id7711893b34263baacac6caf4d489467053131bb
Gerrit-Change-Number: 15560
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190917/1c180746/attachment.htm>