Fix: sessiond: assertion hit in ltt_sessions_ht_empty

[lttng-tools.git] / src / bin / lttng-sessiond / session.cpp

diff --git a/src/bin/lttng-sessiond/session.cpp b/src/bin/lttng-sessiond/session.cpp
index b97f6eb2c81823b3e2491e34445f40efb6d2d138..304589068aaaf6a4d45c4431103b262a6665ff25 100644 (file)
--- a/src/bin/lttng-sessiond/session.cpp
+++ b/src/bin/lttng-sessiond/session.cpp
@@ -17,19 +17,19 @@
 #include <sys/types.h>
 #include <pthread.h>
 
-#include <common/common.h>
-#include <common/utils.h>
-#include <common/trace-chunk.h>
-#include <common/sessiond-comm/sessiond-comm.h>
-#include <lttng/location-internal.h>
-#include "lttng-sessiond.h"
-#include "kernel.h"
-
-#include "session.h"
-#include "utils.h"
-#include "trace-ust.h"
-#include "timer.h"
-#include "cmd.h"
+#include <common/common.hpp>
+#include <common/utils.hpp>
+#include <common/trace-chunk.hpp>
+#include <common/sessiond-comm/sessiond-comm.hpp>
+#include <lttng/location-internal.hpp>
+#include "lttng-sessiond.hpp"
+#include "kernel.hpp"
+
+#include "session.hpp"
+#include "utils.hpp"
+#include "trace-ust.hpp"
+#include "timer.hpp"
+#include "cmd.hpp"
 
 struct ltt_session_destroy_notifier_element {
        ltt_session_destroy_notifier notifier;
@@ -377,24 +377,45 @@ end:
 
 /*
  * Test if ltt_sessions_ht_by_id/name are empty.
- * Return 1 if empty, 0 if not empty.
+ * Return `false` if hash table objects are null.
  * The session list lock must be held.
  */
-static int ltt_sessions_ht_empty(void)
+static bool ltt_sessions_ht_empty(void)
 {
-       unsigned long count;
+       bool empty = false;
 
        if (!ltt_sessions_ht_by_id) {
-               count = 0;
+               /* The hash tables do not exist yet. */
                goto end;
        }
 
        LTTNG_ASSERT(ltt_sessions_ht_by_name);
 
-       count = lttng_ht_get_count(ltt_sessions_ht_by_id);
-       LTTNG_ASSERT(count == lttng_ht_get_count(ltt_sessions_ht_by_name));
+       if (lttng_ht_get_count(ltt_sessions_ht_by_id) == 0) {
+               /* Not empty.*/
+               goto end;
+       }
+
+       /*
+        * At this point it is expected that the `ltt_sessions_ht_by_name` ht is
+        * empty.
+        *
+        * The removal from both hash tables is done in two different
+        * places:
+        *   - removal from `ltt_sessions_ht_by_name` is done during
+        *     `session_destroy()`
+        *   - removal from `ltt_sessions_ht_by_id` is done later
+        *     in `session_release()` on the last reference put.
+        *
+        * This means that it is possible for `ltt_sessions_ht_by_name` to be
+        * empty but for `ltt_sessions_ht_by_id` to still contain objects when
+        * multiple sessions exists. The reverse is false, hence this sanity
+        * check.
+        */
+       LTTNG_ASSERT(lttng_ht_get_count(ltt_sessions_ht_by_name) == 0);
+       empty = true;
 end:
-       return count ? 0 : 1;
+       return empty;
 }
 
 /*
@@ -1143,6 +1164,7 @@ struct ltt_session *session_find_by_id(uint64_t id)
        struct lttng_ht_iter iter;
        struct ltt_session *ls;
 
+       ASSERT_RCU_READ_LOCKED();
        ASSERT_LOCKED(ltt_session_list.lock);
 
        if (!ltt_sessions_ht_by_id) {
@@ -1186,7 +1208,7 @@ enum lttng_error_code session_create(const char *name, uid_t uid, gid_t gid,
                        goto error;
                }
        }
-       new_session = (ltt_session *) zmalloc(sizeof(struct ltt_session));
+       new_session = zmalloc<ltt_session>();
        if (!new_session) {
                PERROR("Failed to allocate an ltt_session structure");
                ret_code = LTTNG_ERR_NOMEM;