From: Jérémie Galarneau Date: Wed, 21 Nov 2018 15:30:15 +0000 (-0500) Subject: Acquire a reference to a session when a timer is active X-Git-Tag: v2.12.0-rc1~736 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=c7031a2c707646804e0088d069578d5eb74fc01e 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 --- diff --git a/src/bin/lttng-sessiond/rotation-thread.c b/src/bin/lttng-sessiond/rotation-thread.c index a9f9967a3..64f958233 100644 --- 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; - uint64_t session_id; + struct ltt_session *session; /* 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(); } - 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( @@ -193,13 +193,14 @@ end: */ 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) { - if (job->session_id == session_id && job->type == job_type) { + if (job->session == session && job->type == job_type) { exists = true; goto end; } @@ -209,7 +210,8 @@ end: } 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 = "!"; @@ -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); - 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. @@ -227,12 +229,15 @@ void rotation_thread_enqueue_job(struct rotation_thread_timer_queue *queue, 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; } + /* 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->session_id = session_id; 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; } - 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; } @@ -731,10 +736,10 @@ int handle_job_queue(struct rotation_thread_handle *handle, pthread_mutex_unlock(&queue->lock); session_lock_list(); - session = session_find_by_id(job->session_id); + session = job->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 diff --git a/src/bin/lttng-sessiond/rotation-thread.h b/src/bin/lttng-sessiond/rotation-thread.h index eb206e701..bb55e07fe 100644 --- 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, - 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); diff --git a/src/bin/lttng-sessiond/timer.c b/src/bin/lttng-sessiond/timer.c index 06d1d4a98..4bff7bd76 100644 --- 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 -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; @@ -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_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"); @@ -214,6 +214,10 @@ int timer_session_rotation_pending_check_start(struct ltt_session *session, { int ret; + if (!session_get(session)) { + ret = -1; + goto end; + } 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, - 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; } - +end: 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; + /* + * The timer's reference to the session can be released safely. + */ + session_put(session); } return ret; } @@ -265,9 +273,13 @@ int timer_session_rotation_schedule_timer_start(struct ltt_session *session, { int ret; + if (!session_get(session)) { + ret = -1; + goto end; + } 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) { @@ -301,6 +313,8 @@ int timer_session_rotation_schedule_timer_stop(struct ltt_session *session) } 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; @@ -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) { + 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, - /* 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, - /* 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); }