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 27f0e472305b6a7a3eb5cafbb50fe8057d0392b5..2a0aab45c962db46f9e9f499a7bdaca3420bd98a 100644 (file)
--- 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;
 
 {
        struct action_executor *executor = _data;
 

+       pthread_mutex_lock(&executor->work.lock);

        executor->should_quit = true;
        pthread_cond_signal(&executor->work.cond);
        executor->should_quit = true;
        pthread_cond_signal(&executor->work.cond);

+       pthread_mutex_unlock(&executor->work.lock);

        return true;
 }
 
        return true;
 }
 


@@ -726,10 +728,10 @@ enum action_executor_status action_executor_enqueue(
        signal = true;
 
 error_unlock:
        signal = true;
 
 error_unlock:

-       pthread_mutex_unlock(&executor->work.lock);

        if (signal) {
                pthread_cond_signal(&executor->work.cond);
        }
        if (signal) {
                pthread_cond_signal(&executor->work.cond);
        }

+       pthread_mutex_unlock(&executor->work.lock);

 
        lttng_evaluation_destroy(evaluation);
        return executor_status;
 
        lttng_evaluation_destroy(evaluation);
        return executor_status;

diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c
index a4d1163399e7cd74975b754b8c51160d1eb189f1..6eda624efad6f784c79cc96f66c5a22c5ceff36b 100644 (file)
--- 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.
                 */
                 * 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);
        }
 }
                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 1a7de1cfd59cf0a095f80794e0ff96d462386a93..9ec5763c1609cb9e77b92afe135b124e56372b53 100644 (file)
--- 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)
 {
 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;
 }
 
        return pthread_cond_broadcast(&stream->metadata_rdv) ? -errno : 0;
 }