Acquire a reference to a session when a timer is active

The timers associated to a session don't directly
reference a session since, up to recently, session
objects were not reference counted. There was
essentially no mechanism in place to prevent a
session from being destroyed while one of its timers
was active.

For this reason, the session was queried by id on
every execution of its timers. However, this did
not prevent the session from being destroyed; it only
allowed the periodic jobs to handle that condition
gracefully.

This change that a reference to the session is held
at all times by periodic jobs that are "in-flight".

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-sessiond/rotation-thread.c b/src/bin/lttng-sessiond/rotation-thread.c
index a9f9967a3b49ee04d3fab5beba7b3d27c268c627..64f958233cdc6ccb22ee671a5d5feb51fd6cae67 100644 (file)
--- a/src/bin/lttng-sessiond/rotation-thread.c
+++ b/src/bin/lttng-sessiond/rotation-thread.c
@@ -56,7 +56,7 @@ struct rotation_thread {
 
 struct rotation_thread_job {
        enum rotation_thread_job_type type;
 
 struct rotation_thread_job {
        enum rotation_thread_job_type type;

-       uint64_t session_id;
+       struct ltt_session *session;

        /* List member in struct rotation_thread_timer_queue. */
        struct cds_list_head head;
 };
        /* List member in struct rotation_thread_timer_queue. */
        struct cds_list_head head;
 };


@@ -131,8 +131,8 @@ void log_job_destruction(const struct rotation_thread_job *job)
                abort();
        }
 
                abort();
        }
 

-       LOG(log_level, "Rotation thread timer queue still contains job of type %s targeting session %" PRIu64 " on destruction",
-                       job_type_str, job->session_id);
+       LOG(log_level, "Rotation thread timer queue still contains job of type %s targeting session \"%s\" on destruction",
+                       job_type_str, job->session->name);

 }
 
 void rotation_thread_timer_queue_destroy(
 }
 
 void rotation_thread_timer_queue_destroy(


@@ -193,13 +193,14 @@ end:
  */
 static
 bool timer_job_exists(const struct rotation_thread_timer_queue *queue,
  */
 static
 bool timer_job_exists(const struct rotation_thread_timer_queue *queue,

-               enum rotation_thread_job_type job_type, uint64_t session_id)
+               enum rotation_thread_job_type job_type,
+               struct ltt_session *session)

 {
        bool exists = false;
        struct rotation_thread_job *job;
 
        cds_list_for_each_entry(job, &queue->list, head) {
 {
        bool exists = false;
        struct rotation_thread_job *job;
 
        cds_list_for_each_entry(job, &queue->list, head) {

-               if (job->session_id == session_id && job->type == job_type) {
+               if (job->session == session && job->type == job_type) {

                        exists = true;
                        goto end;
                }
                        exists = true;
                        goto end;
                }


@@ -209,7 +210,8 @@ end:
 }
 
 void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue,
 }
 
 void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue,

-               enum rotation_thread_job_type job_type, uint64_t session_id)
+               enum rotation_thread_job_type job_type,
+               struct ltt_session *session)

 {
        int ret;
        const char * const dummy = "!";
 {
        int ret;
        const char * const dummy = "!";


@@ -217,7 +219,7 @@ void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue,
        const char *job_type_str = get_job_type_str(job_type);
 
        pthread_mutex_lock(&queue->lock);
        const char *job_type_str = get_job_type_str(job_type);
 
        pthread_mutex_lock(&queue->lock);

-       if (timer_job_exists(queue, session_id, job_type)) {
+       if (timer_job_exists(queue, job_type, session)) {

                /*
                 * This timer job is already pending, we don't need to add
                 * it.
                /*
                 * This timer job is already pending, we don't need to add
                 * it.


@@ -227,12 +229,15 @@ void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue,
 
        job = zmalloc(sizeof(struct rotation_thread_job));
        if (!job) {
 
        job = zmalloc(sizeof(struct rotation_thread_job));
        if (!job) {

-               PERROR("Failed to allocate rotation thread job of type \"%s\" for session id %" PRIu64,
-                               job_type_str, session_id);
+               PERROR("Failed to allocate rotation thread job of type \"%s\" for session \"%s\"",
+                               job_type_str, session->name);

                goto end;
        }
                goto end;
        }

+       /* No reason for this to fail as the caller must hold a reference. */
+       (void) session_get(session);
+
+       job->session = session;

        job->type = job_type;
        job->type = job_type;

-       job->session_id = session_id;

        cds_list_add_tail(&job->head, &queue->list);
 
        ret = lttng_write(lttng_pipe_get_writefd(queue->event_pipe), dummy,
        cds_list_add_tail(&job->head, &queue->list);
 
        ret = lttng_write(lttng_pipe_get_writefd(queue->event_pipe), dummy,


@@ -253,8 +258,8 @@ void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue,
                        DBG("Wake-up pipe of rotation thread job queue is full");
                        goto end;
                }
                        DBG("Wake-up pipe of rotation thread job queue is full");
                        goto end;
                }

-               PERROR("Failed to wake-up the rotation thread after pushing a job of type \"%s\" for session id %" PRIu64,
-                               job_type_str, session_id);
+               PERROR("Failed to wake-up the rotation thread after pushing a job of type \"%s\" for session \"%s\"",
+                               job_type_str, session->name);

                goto end;
        }
 
                goto end;
        }
 


@@ -731,10 +736,10 @@ int handle_job_queue(struct rotation_thread_handle *handle,
                pthread_mutex_unlock(&queue->lock);
 
                session_lock_list();
                pthread_mutex_unlock(&queue->lock);
 
                session_lock_list();

-               session = session_find_by_id(job->session_id);
+               session = job->session;

                if (!session) {
                if (!session) {

-                       DBG("[rotation-thread] Session %" PRIu64 " not found",
-                                       job->session_id);
+                       DBG("[rotation-thread] Session \"%s\" not found",
+                                       session->name);

                        /*
                         * This is a non-fatal error, and we cannot report it to
                         * the user (timer), so just print the error and
                        /*
                         * This is a non-fatal error, and we cannot report it to
                         * the user (timer), so just print the error and

diff --git a/src/bin/lttng-sessiond/rotation-thread.h b/src/bin/lttng-sessiond/rotation-thread.h
index eb206e701ebccf7755009c4136a928e5c0a3da40..bb55e07fe96ff5f40d7825bc7523740b48854c89 100644 (file)
--- a/src/bin/lttng-sessiond/rotation-thread.h
+++ b/src/bin/lttng-sessiond/rotation-thread.h
@@ -54,7 +54,8 @@ void rotation_thread_handle_destroy(
                struct rotation_thread_handle *handle);
 
 void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue,
                struct rotation_thread_handle *handle);
 
 void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue,

-               enum rotation_thread_job_type job_type, uint64_t session_id);
+               enum rotation_thread_job_type job_type,
+               struct ltt_session *session);

 
 void *thread_rotation(void *data);
 
 
 void *thread_rotation(void *data);
 

diff --git a/src/bin/lttng-sessiond/timer.c b/src/bin/lttng-sessiond/timer.c
index 06d1d4a9803213850366699992729f7a26f0e3dd..4bff7bd76cea0497c311f12b7026b0142761710c 100644 (file)
--- a/src/bin/lttng-sessiond/timer.c
+++ b/src/bin/lttng-sessiond/timer.c
@@ -149,7 +149,7 @@ void timer_signal_thread_qs(unsigned int signr)
  * a positive value if no timer was created (not an error).
  */
 static
  * a positive value if no timer was created (not an error).
  */
 static

-int timer_start(timer_t *timer_id, uint64_t session_id,
+int timer_start(timer_t *timer_id, struct ltt_session *session,

                unsigned int timer_interval_us, int signal, bool one_shot)
 {
        int ret = 0, delete_ret;
                unsigned int timer_interval_us, int signal, bool one_shot)
 {
        int ret = 0, delete_ret;


@@ -158,7 +158,7 @@ int timer_start(timer_t *timer_id, uint64_t session_id,
 
        sev.sigev_notify = SIGEV_SIGNAL;
        sev.sigev_signo = signal;
 
        sev.sigev_notify = SIGEV_SIGNAL;
        sev.sigev_signo = signal;

-       sev.sigev_value.sival_ptr = UINT_TO_PTR(session_id);
+       sev.sigev_value.sival_ptr = session;

        ret = timer_create(CLOCK_MONOTONIC, &sev, timer_id);
        if (ret == -1) {
                PERROR("timer_create");
        ret = timer_create(CLOCK_MONOTONIC, &sev, timer_id);
        if (ret == -1) {
                PERROR("timer_create");


@@ -214,6 +214,10 @@ int timer_session_rotation_pending_check_start(struct ltt_session *session,
 {
        int ret;
 
 {
        int ret;
 

+       if (!session_get(session)) {
+               ret = -1;
+               goto end;
+       }

        DBG("Enabling session rotation pending check timer on session %" PRIu64,
                        session->id);
        /*
        DBG("Enabling session rotation pending check timer on session %" PRIu64,
                        session->id);
        /*


@@ -226,13 +230,13 @@ int timer_session_rotation_pending_check_start(struct ltt_session *session,
         * no need to go through the whole signal teardown scheme everytime.
         */
        ret = timer_start(&session->rotation_pending_check_timer,
         * no need to go through the whole signal teardown scheme everytime.
         */
        ret = timer_start(&session->rotation_pending_check_timer,

-                       session->id, interval_us,
+                       session, interval_us,

                        LTTNG_SESSIOND_SIG_PENDING_ROTATION_CHECK,
                        /* one-shot */ true);
        if (ret == 0) {
                session->rotation_pending_check_timer_enabled = true;
        }
                        LTTNG_SESSIOND_SIG_PENDING_ROTATION_CHECK,
                        /* one-shot */ true);
        if (ret == 0) {
                session->rotation_pending_check_timer_enabled = true;
        }

-
+end:

        return ret;
 }
 
        return ret;
 }
 


@@ -253,6 +257,10 @@ int timer_session_rotation_pending_check_stop(struct ltt_session *session)
                ERR("Failed to stop rotate_pending_check timer");
        } else {
                session->rotation_pending_check_timer_enabled = false;
                ERR("Failed to stop rotate_pending_check timer");
        } else {
                session->rotation_pending_check_timer_enabled = false;

+               /*
+                * The timer's reference to the session can be released safely.
+                */
+               session_put(session);

        }
        return ret;
 }
        }
        return ret;
 }


@@ -265,9 +273,13 @@ int timer_session_rotation_schedule_timer_start(struct ltt_session *session,
 {
        int ret;
 
 {
        int ret;
 

+       if (!session_get(session)) {
+               ret = -1;
+               goto end;
+       }

        DBG("Enabling scheduled rotation timer on session \"%s\" (%ui µs)", session->name,
                        interval_us);
        DBG("Enabling scheduled rotation timer on session \"%s\" (%ui µs)", session->name,
                        interval_us);

-       ret = timer_start(&session->rotation_schedule_timer, session->id,
+       ret = timer_start(&session->rotation_schedule_timer, session,

                        interval_us, LTTNG_SESSIOND_SIG_SCHEDULED_ROTATION,
                        /* one-shot */ false);
        if (ret < 0) {
                        interval_us, LTTNG_SESSIOND_SIG_SCHEDULED_ROTATION,
                        /* one-shot */ false);
        if (ret < 0) {


@@ -301,6 +313,8 @@ int timer_session_rotation_schedule_timer_stop(struct ltt_session *session)
        }
 
        session->rotation_schedule_timer_enabled = false;
        }
 
        session->rotation_schedule_timer_enabled = false;

+       /* The timer's reference to the session can be released safely. */
+       session_put(session);

        ret = 0;
 end:
        return ret;
        ret = 0;
 end:
        return ret;


@@ -370,13 +384,25 @@ void *timer_thread_func(void *data)
                } else if (signr == LTTNG_SESSIOND_SIG_EXIT) {
                        goto end;
                } else if (signr == LTTNG_SESSIOND_SIG_PENDING_ROTATION_CHECK) {
                } else if (signr == LTTNG_SESSIOND_SIG_EXIT) {
                        goto end;
                } else if (signr == LTTNG_SESSIOND_SIG_PENDING_ROTATION_CHECK) {

+                       struct ltt_session *session =
+                                       (struct ltt_session *) info.si_value.sival_ptr;
+

                        rotation_thread_enqueue_job(ctx->rotation_thread_job_queue,
                                        ROTATION_THREAD_JOB_TYPE_CHECK_PENDING_ROTATION,
                        rotation_thread_enqueue_job(ctx->rotation_thread_job_queue,
                                        ROTATION_THREAD_JOB_TYPE_CHECK_PENDING_ROTATION,

-                                       /* session_id */ PTR_TO_UINT(info.si_value.sival_ptr));
+                                       session);
+                       session_lock_list();
+                       session_put(session);
+                       session_unlock_list();

                } else if (signr == LTTNG_SESSIOND_SIG_SCHEDULED_ROTATION) {
                        rotation_thread_enqueue_job(ctx->rotation_thread_job_queue,
                                        ROTATION_THREAD_JOB_TYPE_SCHEDULED_ROTATION,
                } else if (signr == LTTNG_SESSIOND_SIG_SCHEDULED_ROTATION) {
                        rotation_thread_enqueue_job(ctx->rotation_thread_job_queue,
                                        ROTATION_THREAD_JOB_TYPE_SCHEDULED_ROTATION,

-                                       /* session_id */ PTR_TO_UINT(info.si_value.sival_ptr));
+                                       (struct ltt_session *) info.si_value.sival_ptr);
+                       /*
+                        * The scheduled periodic rotation timer is not in
+                        * "one-shot" mode. The reference to the session is not
+                        * released since the timer is still enabled and can
+                        * still fire.
+                        */

                } else {
                        ERR("Unexpected signal %d\n", info.si_signo);
                }
                } else {
                        ERR("Unexpected signal %d\n", info.si_signo);
                }