From: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Mon, 30 Nov 2020 19:54:18 +0000 (-0500)
Subject: Fix: missing `_mutex_lock()` before signaling a condition variable
X-Git-Tag: v2.13.0-rc1~407
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=8db3acaf0eb2b0032c2ba25b038d37d166933fa6;p=lttng-tools.git

Fix: missing `_mutex_lock()` before signaling a condition variable

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 <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iebf5a8b2e4251bd1ff4cd462e548cd3486c6cb75
---

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(&ltt_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;
 }