Fix: missing `_mutex_lock()` before signaling a condition variable
authorFrancis Deslauriers <francis.deslauriers@efficios.com>
Mon, 30 Nov 2020 19:54:18 +0000 (14:54 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Tue, 1 Dec 2020 02:16:06 +0000 (21:16 -0500)
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

src/bin/lttng-sessiond/action-executor.c
src/bin/lttng-sessiond/session.c
src/common/ust-consumer/ust-consumer.c

index 27f0e472305b6a7a3eb5cafbb50fe8057d0392b5..2a0aab45c962db46f9e9f499a7bdaca3420bd98a 100644 (file)
@@ -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;
index a4d1163399e7cd74975b754b8c51160d1eb189f1..6eda624efad6f784c79cc96f66c5a22c5ceff36b 100644 (file)
@@ -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);
        }
 }
index 1a7de1cfd59cf0a095f80794e0ff96d462386a93..9ec5763c1609cb9e77b92afe135b124e56372b53 100644 (file)
@@ -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;
 }
 
This page took 0.036918 seconds and 4 git commands to generate.