CDRIVER-4113: Thread-safe pool for server session objects (mongodb#847)

* CXX-4113: Thread-safe pool for server session objects

* Remove unused node pointers on server sessions

* Track pool size, and don't loop while holding the lock to clear

* Cleanup and some redesign of pool API

* Declare special pool for server sessions

* Mention the LIFO nature of the thread-pool, and the non-conformance of the 'push' operation

* auditing the pool is compile-time optional

* Remove -Waggregate-return

Author: vector-of-bool

build/maintainer-flags.txt

@@ -1,5 +1,4 @@
 -Wall
--Waggregate-return
 -Wdeclaration-after-statement
 -Wempty-body
 -Wformat

src/libbson/src/bson/bson-macros.h

@@ -301,5 +301,15 @@
 #define BSON_GNUC_DEPRECATED_FOR(f) BSON_GNUC_DEPRECATED
 #endif
 
+/**
+ * @brief Mark the attached declared entity as "possibly-unused."
+ *
+ * Does nothing on MSVC.
+ */
+#if defined(__GNUC__) || defined(__clang__)
+#define BSON_MAYBE_UNUSED __attribute__ ((unused))
+#else
+#define BSON_MAYBE_UNUSED /* Nothing for other compilers */
+#endif
 
 #endif /* BSON_MACROS_H */

src/libmongoc/CMakeLists.txt

@@ -558,6 +558,7 @@ set (SOURCES ${SOURCES}
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-description.c
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-description-apm.c
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-scanner.c
+   ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-ts-pool.c
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-uri.c
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-util.c
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-version-functions.c
@@ -615,6 +616,7 @@ set (HEADERS
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-gridfs.h
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-socket.h
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-description.h
+   ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-ts-pool-private.h
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-uri.h
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-version-functions.h
    ${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-write-concern.h
@@ -998,6 +1000,7 @@ set (test-libmongoc-sources
    ${PROJECT_SOURCE_DIR}/tests/test-mongoc-topology-scanner.c
    ${PROJECT_SOURCE_DIR}/tests/test-mongoc-topology.c
    ${PROJECT_SOURCE_DIR}/tests/test-mongoc-transactions.c
+   ${PROJECT_SOURCE_DIR}/tests/test-mongoc-ts-pool.c
    ${PROJECT_SOURCE_DIR}/tests/test-mongoc-uri.c
    ${PROJECT_SOURCE_DIR}/tests/test-mongoc-usleep.c
    ${PROJECT_SOURCE_DIR}/tests/test-mongoc-util.c

src/libmongoc/src/mongoc/CMakeLists.txt

@@ -168,6 +168,7 @@ set (src_libmongoc_src_mongoc_DIST_noinst_hs
    mongoc-topology-background-monitoring-private.h
    mongoc-topology-scanner-private.h
    mongoc-trace-private.h
+   mongoc-ts-pool-private.h
    mongoc-uri-private.h
    mongoc-util-private.h
    mongoc-write-command-private.h
@@ -253,6 +254,7 @@ set (src_libmongoc_src_mongoc_DIST_cs
    mongoc-topology-description.c
    mongoc-topology-description-apm.c
    mongoc-topology-scanner.c
+   mongoc-ts-pool.c
    mongoc-uri.c
    mongoc-util.c
    mongoc-version-functions.c

src/libmongoc/src/mongoc/mongoc-client-pool.c

@@ -185,7 +185,7 @@ mongoc_client_pool_destroy (mongoc_client_pool_t *pool)
       EXIT;
    }
 
-   if (pool->topology->session_pool) {
+   if (!mongoc_server_session_pool_is_empty (pool->topology->session_pool)) {
       client = mongoc_client_pool_pop (pool);
       _mongoc_client_end_sessions (client);
       mongoc_client_pool_push (pool, client);

src/libmongoc/src/mongoc/mongoc-client-session-private.h

@@ -45,7 +45,6 @@ struct _mongoc_session_opt_t {
 };
 
 typedef struct _mongoc_server_session_t {
-   struct _mongoc_server_session_t *prev, *next;
    int64_t last_used_usec;
    bson_t lsid;        /* logical session id */
    int64_t txn_number; /* transaction number */
@@ -102,16 +101,17 @@ _mongoc_client_session_handle_reply (mongoc_client_session_t *session,
                                      const char *cmd_name,
                                      const bson_t *reply);
 
-mongoc_server_session_t *
-_mongoc_server_session_new (bson_error_t *error);
+bool
+_mongoc_server_session_init (mongoc_server_session_t *session,
+                             bson_error_t *error);
+
+void
+_mongoc_server_session_destroy (mongoc_server_session_t *session);
 
 bool
 _mongoc_server_session_timed_out (const mongoc_server_session_t *server_session,
                                   int64_t session_timeout_minutes);
 
-void
-_mongoc_server_session_destroy (mongoc_server_session_t *server_session);
-
 mongoc_client_session_t *
 _mongoc_client_session_new (mongoc_client_t *client,
                             mongoc_server_session_t *server_session,

src/libmongoc/src/mongoc/mongoc-client-session.c

@@ -709,35 +709,26 @@ _mongoc_client_session_handle_reply (mongoc_client_session_t *session,
    }
 }
 
-
-mongoc_server_session_t *
-_mongoc_server_session_new (bson_error_t *error)
+bool
+_mongoc_server_session_init (mongoc_server_session_t *self, bson_error_t *error)
 {
    uint8_t uuid_data[16];
-   mongoc_server_session_t *s;
-
    ENTRY;
-
+   BSON_ASSERT (self);
    if (!_mongoc_server_session_uuid (uuid_data, error)) {
-      RETURN (NULL);
+      RETURN (false);
    }
-
-   s = bson_malloc0 (sizeof (mongoc_server_session_t));
-   s->last_used_usec = SESSION_NEVER_USED;
-   s->prev = NULL;
-   s->next = NULL;
-   bson_init (&s->lsid);
-   bson_append_binary (
-      &s->lsid, "id", 2, BSON_SUBTYPE_UUID, uuid_data, sizeof uuid_data);
-
    /* transaction number is a positive integer and will be incremented before
     * each use, so ensure it is initialized to zero. */
-   s->txn_number = 0;
+   self->txn_number = 0;
+   self->last_used_usec = SESSION_NEVER_USED;
+   bson_init (&self->lsid);
+   BSON_APPEND_BINARY (
+      &self->lsid, "id", BSON_SUBTYPE_UUID, uuid_data, sizeof uuid_data);
 
-   RETURN (s);
+   RETURN (true);
 }
 
-
 bool
 _mongoc_server_session_timed_out (const mongoc_server_session_t *server_session,
                                   int64_t session_timeout_minutes)
@@ -766,14 +757,9 @@ _mongoc_server_session_timed_out (const mongoc_server_session_t *server_session,
 
 
 void
-_mongoc_server_session_destroy (mongoc_server_session_t *server_session)
+_mongoc_server_session_destroy (mongoc_server_session_t *self)
 {
-   ENTRY;
-
-   bson_destroy (&server_session->lsid);
-   bson_free (server_session);
-
-   EXIT;
+   bson_destroy (&self->lsid);
 }
 
 
@@ -806,7 +792,8 @@ _mongoc_client_session_new (mongoc_client_t *client,
                  DEFAULT_MAX_COMMIT_TIME_MS);
 
    if (opts) {
-      mongoc_optional_copy (&opts->causal_consistency, &session->opts.causal_consistency);
+      mongoc_optional_copy (&opts->causal_consistency,
+                            &session->opts.causal_consistency);
       mongoc_optional_copy (&opts->snapshot, &session->opts.snapshot);
       txn_opts_set (&session->opts.default_txn_opts,
                     opts->default_txn_opts.read_concern,
@@ -1684,9 +1671,9 @@ mongoc_client_session_destroy (mongoc_client_session_t *session)
       _mongoc_client_push_server_session (session->client,
                                           session->server_session);
    } else {
-      /* If the client has been reset, destroy the server session instead of
-    pushing it back into the topology's pool. */
-      _mongoc_server_session_destroy (session->server_session);
+      /** If the client has been reset, destroy the server session instead of
+       * pushing it back into the topology's pool. */
+      mongoc_server_session_pool_drop (session->server_session);
    }
 
    txn_opts_cleanup (&session->opts.default_txn_opts);

src/libmongoc/src/mongoc/mongoc-client.c

@@ -3013,7 +3013,7 @@ _mongoc_client_end_sessions (mongoc_client_t *client)
    mongoc_cluster_t *cluster = &client->cluster;
    bool r;
 
-   if (t->session_pool) {
+   while (!mongoc_server_session_pool_is_empty (t->session_pool)) {
       prefs = mongoc_read_prefs_new (MONGOC_READ_PRIMARY_PREFERRED);
       server_id =
          mongoc_topology_select_server_id (t, MONGOC_SS_READ, prefs, &error);
@@ -3087,7 +3087,7 @@ mongoc_client_reset (mongoc_client_t *client)
    client->client_sessions = mongoc_set_new (8, NULL, NULL);
 
    /* Server sessions are owned by us, so we clear the pool on reset. */
-   _mongoc_topology_clear_session_pool (client->topology);
+   mongoc_server_session_pool_clear (client->topology->session_pool);
 }
 
 mongoc_change_stream_t *

src/libmongoc/src/mongoc/mongoc-topology-private.h

@@ -27,6 +27,7 @@
 #include "mongoc-uri.h"
 #include "mongoc-client-session-private.h"
 #include "mongoc-crypt-private.h"
+#include "mongoc-ts-pool-private.h"
 
 #define MONGOC_TOPOLOGY_MIN_HEARTBEAT_FREQUENCY_MS 500
 #define MONGOC_TOPOLOGY_SOCKET_CHECK_INTERVAL_MS 5000
@@ -62,6 +63,17 @@ typedef struct _mongoc_rr_data_t {
    char *txt_record_opts;
 } mongoc_rr_data_t;
 
+struct _mongoc_topology_t;
+
+MONGOC_DECL_SPECIAL_TS_POOL (
+   mongoc_server_session_t,
+   mongoc_server_session_pool,
+   struct _mongoc_topology_t,
+   /* ctor/dtor/prune are defined in the new_with_params call */
+   NULL,
+   NULL,
+   NULL)
+
 typedef bool (*_mongoc_rr_resolver_fn) (const char *service,
                                         mongoc_rr_type_t rr_type,
                                         mongoc_rr_data_t *rr_data,
@@ -102,7 +114,7 @@ typedef struct _mongoc_topology_t {
    bool single_threaded;
    bool stale;
 
-   mongoc_server_session_t *session_pool;
+   mongoc_server_session_pool session_pool;
 
    /* Is client side encryption enabled? */
    bool cse_enabled;
@@ -215,9 +227,6 @@ _mongoc_topology_push_server_session (mongoc_topology_t *topology,
 bool
 _mongoc_topology_end_sessions_cmd (mongoc_topology_t *topology, bson_t *cmd);
 
-void
-_mongoc_topology_clear_session_pool (mongoc_topology_t *topology);
-
 void
 _mongoc_topology_do_blocking_scan (mongoc_topology_t *topology,
                                    bson_error_t *error);
@@ -255,7 +264,7 @@ _mongoc_topology_handle_app_error (mongoc_topology_t *topology,
 void
 _mongoc_topology_clear_connection_pool (mongoc_topology_t *topology,
                                         uint32_t server_id,
-                                        const bson_oid_t* service_id);
+                                        const bson_oid_t *service_id);
 
 void
 mongoc_topology_rescan_srv (mongoc_topology_t *topology);