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 2eaca237ff9f60708edbe57b3f40daaac6ba42b2..ea760000c39febfd11856963b4a4a4485ae31b5e 100644 (file)
--- a/src/bin/lttng-sessiond/consumer.c
+++ b/src/bin/lttng-sessiond/consumer.c
@@ -472,6 +472,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);
 
@@ -506,11 +507,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);
 
@@ -522,6 +522,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.
  *
@@ -530,33 +551,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 a9266d5b939798882377bc2caeb0e67251464fa2..ebff43f9db93cc38059b892ece881e8ceb154412 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 6545a5ef21d3d99bbb39c81587839380bed06ee3..19283d439087c7a155656314803b0472e093b0a7 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -2634,7 +2634,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);
@@ -2647,7 +2647,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 cd59cb7ab0cec8624992a8ddb90da63e365388fb..452438952adb580f265e9d563d7be68d406d630b 100644 (file)
--- a/src/bin/lttng-sessiond/session.c
+++ b/src/bin/lttng-sessiond/session.c
@@ -195,7 +195,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 e5660dac7b02f13bb59c49cabfdd93ba3952ccdf..07a31e080e8c5d9bf049167838e40151f7872e76 100644 (file)
--- a/src/bin/lttng-sessiond/snapshot.c
+++ b/src/bin/lttng-sessiond/snapshot.c
@@ -222,7 +222,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 093297cb01ebc1216855401199b829b5a5659da2..9b4fc9015501b08b3aca97db1319ae003c201b4d 100644 (file)
--- a/src/bin/lttng-sessiond/trace-kernel.c
+++ b/src/bin/lttng-sessiond/trace-kernel.c
@@ -108,14 +108,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:
@@ -515,8 +507,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 db91b2ed24cd57191e92a32376f0bc94e6b47482..d7763f65669be3c1e662e24416f846d5b43825c3 100644 (file)
--- a/src/bin/lttng-sessiond/trace-kernel.h
+++ b/src/bin/lttng-sessiond/trace-kernel.h
@@ -100,14 +100,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 5f41bd764308a812806f623bdfb2ed24c09b838d..c84b0edba3428f530d2f0332c425886891d81f81 100644 (file)
--- a/src/bin/lttng-sessiond/trace-ust.c
+++ b/src/bin/lttng-sessiond/trace-ust.c
@@ -281,14 +281,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;
@@ -781,8 +773,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);
 
        free(session);
 }

diff --git a/src/bin/lttng-sessiond/trace-ust.h b/src/bin/lttng-sessiond/trace-ust.h
index 795389e9da3f34e55c6eae7fc7bffc46c4b09c80..674c2e29247c4f4d1ba3779bbefbcbadc72318f3 100644 (file)
--- a/src/bin/lttng-sessiond/trace-ust.h
+++ b/src/bin/lttng-sessiond/trace-ust.h
@@ -85,14 +85,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 0ee7790b95f687ada99df6d5021c148b471d9115..c1fe2f2a517748fedad70a04f40d17dea96bc4db 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -721,6 +721,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);
 }
 
@@ -1645,8 +1647,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,
@@ -1701,9 +1706,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);
 }
 
 /*