Fix: reference counting of consumer output

The UST app session has a reference on the consumer output object, but
it belongs to the UST session. Implement a refcounting scheme to ensure
it is not freed before all users are done using it.

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

diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c
index 87d5f34387befba860e31feefe0059a9c8c12f9b..41ad46d8689338f3c27c3fc80fb12dd46bbdb53f 100644 (file)
--- a/src/bin/lttng-sessiond/consumer.c
+++ b/src/bin/lttng-sessiond/consumer.c
@@ -473,6 +473,7 @@ struct consumer_output *consumer_create_output(enum consumer_dst_type type)
        output->enabled = 1;
        output->type = type;
        output->net_seq_index = (uint64_t) -1ULL;
+       urcu_ref_init(&output->ref);
 
        output->socks = lttng_ht_new(0, LTTNG_HT_TYPE_ULONG);
 
@@ -507,11 +508,10 @@ void consumer_destroy_output_sockets(struct consumer_output *obj)
  *
  * Should *NOT* be called with RCU read-side lock held.
  */
-void consumer_destroy_output(struct consumer_output *obj)
+static void consumer_release_output(struct urcu_ref *ref)
 {
-       if (obj == NULL) {
-               return;
-       }
+       struct consumer_output *obj =
+               caa_container_of(ref, struct consumer_output, ref);
 
        consumer_destroy_output_sockets(obj);
 
@@ -523,6 +523,27 @@ void consumer_destroy_output(struct consumer_output *obj)
        free(obj);
 }
 
+/*
+ * Get the consumer_output object.
+ */
+void consumer_output_get(struct consumer_output *obj)
+{
+       urcu_ref_get(&obj->ref);
+}
+
+/*
+ * Put the consumer_output object.
+ *
+ * Should *NOT* be called with RCU read-side lock held.
+ */
+void consumer_output_put(struct consumer_output *obj)
+{
+       if (!obj) {
+               return;
+       }
+       urcu_ref_put(&obj->ref, consumer_release_output);
+}
+
 /*
  * Copy consumer output and returned the newly allocated copy.
  *
@@ -531,33 +552,28 @@ void consumer_destroy_output(struct consumer_output *obj)
 struct consumer_output *consumer_copy_output(struct consumer_output *obj)
 {
        int ret;
-       struct lttng_ht *tmp_ht_ptr;
        struct consumer_output *output;
 
        assert(obj);
 
        output = consumer_create_output(obj->type);
        if (output == NULL) {
-               goto error;
+               goto end;
        }
-       /* Avoid losing the HT reference after the memcpy() */
-       tmp_ht_ptr = output->socks;
-
-       memcpy(output, obj, sizeof(struct consumer_output));
-
-       /* Putting back the HT pointer and start copying socket(s). */
-       output->socks = tmp_ht_ptr;
-
+       output->enabled = obj->enabled;
+       output->net_seq_index = obj->net_seq_index;
+       memcpy(output->subdir, obj->subdir, PATH_MAX);
+       output->snapshot = obj->snapshot;
+       memcpy(&output->dst, &obj->dst, sizeof(output->dst));
        ret = consumer_copy_sockets(output, obj);
        if (ret < 0) {
-               goto malloc_error;
+               goto error_put;
        }
-
-error:
+end:
        return output;
 
-malloc_error:
-       consumer_destroy_output(output);
+error_put:
+       consumer_output_put(output);
        return NULL;
 }
 

diff --git a/src/bin/lttng-sessiond/consumer.h b/src/bin/lttng-sessiond/consumer.h
index e4845f5272904dc85d52669ae26740beadb93e07..73f113d0aca6fd8f4d33e2ba60cfe9bedd398d9f 100644 (file)
--- a/src/bin/lttng-sessiond/consumer.h
+++ b/src/bin/lttng-sessiond/consumer.h
@@ -21,6 +21,7 @@
 #include <common/consumer.h>
 #include <common/hashtable/hashtable.h>
 #include <lttng/lttng.h>
+#include <urcu/ref.h>
 
 #include "snapshot.h"
 
@@ -140,6 +141,8 @@ struct consumer_net {
  * Consumer output object describing where and how to send data.
  */
 struct consumer_output {
+       struct urcu_ref ref;    /* Refcount */
+
        /* If the consumer is enabled meaning that should be used */
        unsigned int enabled;
        enum consumer_dst_type type;
@@ -192,7 +195,8 @@ int consumer_socket_recv(struct consumer_socket *socket, void *msg,
 
 struct consumer_output *consumer_create_output(enum consumer_dst_type type);
 struct consumer_output *consumer_copy_output(struct consumer_output *obj);
-void consumer_destroy_output(struct consumer_output *obj);
+void consumer_output_get(struct consumer_output *obj);
+void consumer_output_put(struct consumer_output *obj);
 int consumer_set_network_uri(struct consumer_output *obj,
                struct lttng_uri *uri);
 int consumer_send_fds(struct consumer_socket *sock, int *fds, size_t nb_fd);

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index ba35726b842b77e0712940d2154712dbbcb09c9a..a78f1af653e2327873f6d5e6bbf93d3647e4bae8 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -2713,7 +2713,7 @@ static int copy_session_consumer(int domain, struct ltt_session *session)
                 * domain.
                 */
                if (session->kernel_session->consumer) {
-                       consumer_destroy_output(session->kernel_session->consumer);
+                       consumer_output_put(session->kernel_session->consumer);
                }
                session->kernel_session->consumer =
                        consumer_copy_output(session->consumer);
@@ -2727,7 +2727,7 @@ static int copy_session_consumer(int domain, struct ltt_session *session)
        case LTTNG_DOMAIN_UST:
                DBG3("Copying tracing session consumer output in UST session");
                if (session->ust_session->consumer) {
-                       consumer_destroy_output(session->ust_session->consumer);
+                       consumer_output_put(session->ust_session->consumer);
                }
                session->ust_session->consumer =
                        consumer_copy_output(session->consumer);

diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c
index b82a330e92da376aef158bddfd8448f925c7a603..49055fb109be531af3354afcb28249b20a623d06 100644 (file)
--- a/src/bin/lttng-sessiond/session.c
+++ b/src/bin/lttng-sessiond/session.c
@@ -198,7 +198,7 @@ int session_destroy(struct ltt_session *session)
        del_session_list(session);
        pthread_mutex_destroy(&session->lock);
 
-       consumer_destroy_output(session->consumer);
+       consumer_output_put(session->consumer);
        snapshot_destroy(&session->snapshot);
        free(session);
 

diff --git a/src/bin/lttng-sessiond/snapshot.c b/src/bin/lttng-sessiond/snapshot.c
index f9366b36e5cab2ea27f8474af7286a4437a7d050..898f77af98f1c463c3682757ef278acbd7da97d2 100644 (file)
--- a/src/bin/lttng-sessiond/snapshot.c
+++ b/src/bin/lttng-sessiond/snapshot.c
@@ -223,7 +223,7 @@ void snapshot_output_destroy(struct snapshot_output *obj)
 
        if (obj->consumer) {
                consumer_output_send_destroy_relayd(obj->consumer);
-               consumer_destroy_output(obj->consumer);
+               consumer_output_put(obj->consumer);
        }
        free(obj);
 }

diff --git a/src/bin/lttng-sessiond/trace-kernel.c b/src/bin/lttng-sessiond/trace-kernel.c
index b86bdfe6060b37c0698f805f2daa89491d54101d..b4dc1b9e81e46765f523cda523a3c26a864dd106 100644 (file)
--- a/src/bin/lttng-sessiond/trace-kernel.c
+++ b/src/bin/lttng-sessiond/trace-kernel.c
@@ -162,14 +162,6 @@ struct ltt_kernel_session *trace_kernel_create_session(void)
                goto error;
        }
 
-       /*
-        * The tmp_consumer stays NULL until a set_consumer_uri command is
-        * executed. At this point, the consumer should be nullify until an
-        * enable_consumer command. This assignment is symbolic since we've zmalloc
-        * the struct.
-        */
-       lks->tmp_consumer = NULL;
-
        return lks;
 
 error:
@@ -578,8 +570,7 @@ void trace_kernel_destroy_session(struct ltt_kernel_session *session)
        }
 
        /* Wipe consumer output object */
-       consumer_destroy_output(session->consumer);
-       consumer_destroy_output(session->tmp_consumer);
+       consumer_output_put(session->consumer);
 
        free(session);
 }

diff --git a/src/bin/lttng-sessiond/trace-kernel.h b/src/bin/lttng-sessiond/trace-kernel.h
index 9d50f4c79f86dd40a30f674c6cce8b5a6a35451b..b9bcbfa77d242d46637e229d552943819b1be371 100644 (file)
--- a/src/bin/lttng-sessiond/trace-kernel.h
+++ b/src/bin/lttng-sessiond/trace-kernel.h
@@ -103,14 +103,7 @@ struct ltt_kernel_session {
        /* UID/GID of the user owning the session */
        uid_t uid;
        gid_t gid;
-       /*
-        * Two consumer_output object are needed where one is needed for the
-        * current output object and the second one is the temporary object used to
-        * store URI being set by the lttng_set_consumer_uri call. Once
-        * lttng_enable_consumer is called, the two pointers are swapped.
-        */
        struct consumer_output *consumer;
-       struct consumer_output *tmp_consumer;
        /* Tracing session id */
        uint64_t id;
        /* Session is active or not meaning it has been started or stopped. */

diff --git a/src/bin/lttng-sessiond/trace-ust.c b/src/bin/lttng-sessiond/trace-ust.c
index 8ae8dfdb1a91bb69b43ccb54747cee8a91d76fa8..b4dcfe37dde72ca66ed573cf50571a0f46c1bfe4 100644 (file)
--- a/src/bin/lttng-sessiond/trace-ust.c
+++ b/src/bin/lttng-sessiond/trace-ust.c
@@ -283,14 +283,6 @@ struct ltt_ust_session *trace_ust_create_session(uint64_t session_id)
                goto error_consumer;
        }
 
-       /*
-        * The tmp_consumer stays NULL until a set_consumer_uri command is
-        * executed. At this point, the consumer should be nullify until an
-        * enable_consumer command. This assignment is symbolic since we've zmalloc
-        * the struct.
-        */
-       lus->tmp_consumer = NULL;
-
        DBG2("UST trace session create successful");
 
        return lus;
@@ -1072,8 +1064,7 @@ void trace_ust_destroy_session(struct ltt_ust_session *session)
                buffer_reg_uid_destroy(reg, session->consumer);
        }
 
-       consumer_destroy_output(session->consumer);
-       consumer_destroy_output(session->tmp_consumer);
+       consumer_output_put(session->consumer);
 
        fini_pid_tracker(&session->pid_tracker);
 

diff --git a/src/bin/lttng-sessiond/trace-ust.h b/src/bin/lttng-sessiond/trace-ust.h
index 343ce3e49b84f2a000dc5f67b9f5d8d138e911f7..1ad96edaa012be3dcd6e3734e7c0ae79cba78e04 100644 (file)
--- a/src/bin/lttng-sessiond/trace-ust.h
+++ b/src/bin/lttng-sessiond/trace-ust.h
@@ -99,14 +99,7 @@ struct ltt_ust_session {
        gid_t gid;
        /* Is the session active meaning has is been started or stopped. */
        unsigned int active:1;
-       /*
-        * Two consumer_output object are needed where one is for the current
-        * output object and the second one is the temporary object used to store
-        * URI being set by the lttng_set_consumer_uri call. Once
-        * lttng_enable_consumer is called, the two pointers are swapped.
-        */
        struct consumer_output *consumer;
-       struct consumer_output *tmp_consumer;
        /* Sequence number for filters so the tracer knows the ordering. */
        uint64_t filter_seq_num;
        /* This indicates which type of buffer this session is set for. */

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index d1cc39baefc4b3fba85b0f6b04dd8b25d9522e46..01204beb7df8f7e945a5e9468db37954c430e154 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -722,6 +722,8 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess,
        }
        pthread_mutex_unlock(&ua_sess->lock);
 
+       consumer_output_put(ua_sess->consumer);
+
        call_rcu(&ua_sess->rcu_head, delete_ust_app_session_rcu);
 }
 
@@ -1685,8 +1687,11 @@ static void shadow_copy_session(struct ust_app_session *ua_sess,
        ua_sess->egid = usess->gid;
        ua_sess->buffer_type = usess->buffer_type;
        ua_sess->bits_per_long = app->bits_per_long;
+
        /* There is only one consumer object per session possible. */
+       consumer_output_get(usess->consumer);
        ua_sess->consumer = usess->consumer;
+
        ua_sess->output_traces = usess->output_traces;
        ua_sess->live_timer_interval = usess->live_timer_interval;
        copy_channel_attr_to_ustctl(&ua_sess->metadata_attr,
@@ -1773,9 +1778,10 @@ static void shadow_copy_session(struct ust_app_session *ua_sess,
 
                lttng_ht_add_unique_str(ua_sess->channels, &ua_chan->node);
        }
+       return;
 
 error:
-       return;
+       consumer_output_put(ua_sess->consumer);
 }
 
 /*