From 8db3acaf0eb2b0032c2ba25b038d37d166933fa6 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Mon, 30 Nov 2020 14:54:18 -0500 Subject: [PATCH] Fix: missing `_mutex_lock()` before signaling a condition variable MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit According to the PTHREAD_COND(3) man page, a condition variable signaling and broadcast should alway be protected with a mutex. This commit fixes two calls to `pthread_cond_signal()` function without holding the right lock. This commit also adds an assertion right before two calls to `pthread_cond_broadcast()` where it's less obvious from the surrounding code that the mutex is held. This documents the code and may be useful for future debugging. Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Change-Id: Iebf5a8b2e4251bd1ff4cd462e548cd3486c6cb75 --- src/bin/lttng-sessiond/action-executor.c | 4 +++- src/bin/lttng-sessiond/session.c | 1 + src/common/ust-consumer/ust-consumer.c | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bin/lttng-sessiond/action-executor.c b/src/bin/lttng-sessiond/action-executor.c index 27f0e4723..2a0aab45c 100644 --- a/src/bin/lttng-sessiond/action-executor.c +++ b/src/bin/lttng-sessiond/action-executor.c @@ -601,8 +601,10 @@ static bool shutdown_action_executor_thread(void *_data) { struct action_executor *executor = _data; + pthread_mutex_lock(&executor->work.lock); executor->should_quit = true; pthread_cond_signal(&executor->work.cond); + pthread_mutex_unlock(&executor->work.lock); return true; } @@ -726,10 +728,10 @@ enum action_executor_status action_executor_enqueue( signal = true; error_unlock: - pthread_mutex_unlock(&executor->work.lock); if (signal) { pthread_cond_signal(&executor->work.cond); } + pthread_mutex_unlock(&executor->work.lock); lttng_evaluation_destroy(evaluation); return executor_status; diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c index a4d116339..6eda624ef 100644 --- a/src/bin/lttng-sessiond/session.c +++ b/src/bin/lttng-sessiond/session.c @@ -989,6 +989,7 @@ void session_release(struct urcu_ref *ref) * Broadcast after free-ing to ensure the memory is * reclaimed before the main thread exits. */ + ASSERT_LOCKED(ltt_session_list.lock); pthread_cond_broadcast(<t_session_list.removal_cond); } } diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index 1a7de1cfd..9ec5763c1 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -2966,6 +2966,7 @@ static int put_next_subbuffer(struct lttng_consumer_stream *stream, static int signal_metadata(struct lttng_consumer_stream *stream, struct lttng_consumer_local_data *ctx) { + ASSERT_LOCKED(stream->metadata_rdv_lock); return pthread_cond_broadcast(&stream->metadata_rdv) ? -errno : 0; } -- 2.34.1