lib-ldap: Added initial connection pooling code.

This is mainly about allowing multiple dict-ldaps to use the same
ldap-connection. In future we could support load balancing with multiple
concurrent LDAP connections.

diff --git a/src/lib-dict-extra/dict-ldap.c b/src/lib-dict-extra/dict-ldap.c
index 9673d92db948429578159af1c84bf36457058f93..616b44b40561bdca00d4d03ecdeda705db6472cc 100644 (file)
--- a/src/lib-dict-extra/dict-ldap.c
+++ b/src/lib-dict-extra/dict-ldap.c
@@ -437,6 +437,7 @@ void dict_ldap_init(struct module *module ATTR_UNUSED)
 
 void dict_ldap_deinit(void)
 {
+       ldap_clients_cleanup();
        dict_driver_unregister(&dict_driver_ldap);
 }
 

diff --git a/src/lib-ldap/Makefile.am b/src/lib-ldap/Makefile.am
index fa8e213fb34cb09eb18ded7098b52c04ec0b2b63..b990e93fdf3b7072052877e471c36fb58302ee86 100644 (file)
--- a/src/lib-ldap/Makefile.am
+++ b/src/lib-ldap/Makefile.am
@@ -11,6 +11,7 @@ AM_CPPFLAGS = \
 libdovecot_ldap_la_SOURCES = \
        ldap-client.c \
        ldap-connection.c \
+       ldap-connection-pool.c \
        ldap-iterator.c \
        ldap-search.c \
        ldap-compare.c \
@@ -24,6 +25,7 @@ headers = \
        ldap-client.h
 
 noinst_HEADERS = \
+       ldap-connection-pool.h \
        ldap-private.h
 
 pkginc_libdir=$(pkgincludedir)

diff --git a/src/lib-ldap/ldap-client.c b/src/lib-ldap/ldap-client.c
index d8b9743dbeabf8f131dee1b08b0efcb67af0d2ce..bdafd87b115d233874f0d7f46f08ff38c80169ee 100644 (file)
--- a/src/lib-ldap/ldap-client.c
+++ b/src/lib-ldap/ldap-client.c
@@ -1,21 +1,31 @@
 /* Copyright (c) 2016 Dovecot authors, see the included COPYING file */
 
 #include "lib.h"
+#include "ldap-connection-pool.h"
 #include "ldap-private.h"
 
+/* Max number of ldap-connections that can be created. For now this is
+   unlimited since we're assuming our callers aren't calling us with many
+   different settings. */
+#define LDAP_CONN_POOL_MAX_CONNECTIONS UINT_MAX
+
 struct ldap_client {
-       /* for now we support just a single connection, but this could be
-          extended to a connection pool. */
-       struct ldap_connection *conn;
+       struct ldap_connection_list *list;
 };
 
+static struct ldap_connection_pool *ldap_conn_pool = NULL;
+
 int ldap_client_init(const struct ldap_client_settings *set,
                     struct ldap_client **client_r, const char **error_r)
 {
        struct ldap_client *client;
 
+       if (ldap_conn_pool == NULL)
+               ldap_conn_pool = ldap_connection_pool_init(LDAP_CONN_POOL_MAX_CONNECTIONS);
+
        client = i_new(struct ldap_client, 1);
-       if (ldap_connection_init(client, set, &client->conn, error_r) < 0) {
+       if (ldap_connection_pool_get(ldap_conn_pool, client, set,
+                                    &client->list, error_r) < 0) {
                i_free(client);
                return -1;
        }
@@ -29,13 +39,13 @@ void ldap_client_deinit(struct ldap_client **_client)
 
        *_client = NULL;
 
-       ldap_connection_deinit(&client->conn);
+       ldap_connection_pool_unref(ldap_conn_pool, &client->list);
        i_free(client);
 }
 
 void ldap_client_switch_ioloop(struct ldap_client *client)
 {
-       ldap_connection_switch_ioloop(client->conn);
+       ldap_connection_switch_ioloop(client->list->conn);
 }
 
 #undef ldap_search_start
@@ -43,7 +53,9 @@ void ldap_search_start(struct ldap_client *client,
                       const struct ldap_search_input *input,
                       ldap_result_callback_t *callback, void *context)
 {
-       return ldap_connection_search_start(client->conn, input, callback, context);
+       /* FIXME: we could support multiple concurrent LDAP connections to
+          the same host. */
+       return ldap_connection_search_start(client->list->conn, input, callback, context);
 }
 
 #undef ldap_compare_start
@@ -51,5 +63,12 @@ void ldap_compare_start(struct ldap_client *client,
                        const struct ldap_compare_input *input,
                        ldap_result_callback_t *callback, void *context)
 {
-       return ldap_connection_compare_start(client->conn, input, callback, context);
+       return ldap_connection_compare_start(client->list->conn, input, callback, context);
+}
+
+void ldap_clients_cleanup(void)
+{
+       if (ldap_conn_pool != NULL &&
+           !ldap_connection_pool_have_references(ldap_conn_pool))
+               ldap_connection_pool_deinit(&ldap_conn_pool);
 }

diff --git a/src/lib-ldap/ldap-client.h b/src/lib-ldap/ldap-client.h
index 5ce14dc6bf7795a5fab4af5f635ac2a6c5115104..f0707c87fbe32e23350b1cf3f20f0086dfecdf19 100644 (file)
--- a/src/lib-ldap/ldap-client.h
+++ b/src/lib-ldap/ldap-client.h
@@ -19,6 +19,8 @@ struct ldap_entry;
 typedef void ldap_result_callback_t(struct ldap_result *result, void *context);
 
 struct ldap_client_settings {
+       /* NOTE: when adding here, remember to update
+          ldap_connection_have_settings() and ldap_connection_init() */
        const char *uri;
        const char *bind_dn;
        const char *password;
@@ -58,6 +60,11 @@ int ldap_client_init(const struct ldap_client_settings *set,
 void ldap_client_deinit(struct ldap_client **client);
 void ldap_client_switch_ioloop(struct ldap_client *client);
 
+/* Deinitialize all pooled LDAP connections if there are no references left.
+   This allows freeing the memory at deinit, but still allows multiple
+   independent code parts to use lib-ldap and call this function. */
+void ldap_clients_cleanup(void);
+
 void ldap_search_start(struct ldap_client *client,
                       const struct ldap_search_input *input,
                       ldap_result_callback_t *callback,

diff --git a/src/lib-ldap/ldap-connection-pool.c b/src/lib-ldap/ldap-connection-pool.c
new file mode 100644 (file)
index 0000000..8b2d7a9
--- /dev/null
+++ b/src/lib-ldap/ldap-connection-pool.c
@@ -0,0 +1,113 @@
+/* Copyright (c) 2016 Dovecot authors, see the included COPYING file */
+
+#include "lib.h"
+#include "llist.h"
+#include "ldap-private.h"
+#include "ldap-connection-pool.h"
+
+struct ldap_connection_pool {
+       struct ldap_connection_list *conn_list;
+       unsigned int conn_count;
+
+       unsigned int max_connections;
+};
+
+static void ldap_connection_list_remove(struct ldap_connection_pool *pool,
+                                       struct ldap_connection_list *list)
+{
+       DLLIST_REMOVE(&pool->conn_list, list);
+       pool->conn_count--;
+
+       ldap_connection_deinit(&list->conn);
+       i_free(list);
+}
+
+static void
+ldap_connection_pool_shrink_to(struct ldap_connection_pool *pool,
+                              unsigned int max_count)
+{
+       struct ldap_connection_list *list, *next;
+
+       list = pool->conn_list;
+       for (; list != NULL && pool->conn_count > max_count; list = next) {
+               next = list->next;
+               if (list->refcount == 0)
+                       ldap_connection_list_remove(pool, list);
+       }
+}
+
+struct ldap_connection_pool *
+ldap_connection_pool_init(unsigned int max_connections)
+{
+       struct ldap_connection_pool *pool;
+
+       pool = i_new(struct ldap_connection_pool, 1);
+       pool->max_connections = max_connections;
+       return pool;
+}
+
+void ldap_connection_pool_deinit(struct ldap_connection_pool **_pool)
+{
+       struct ldap_connection_pool *pool = *_pool;
+
+       *_pool = NULL;
+
+       ldap_connection_pool_shrink_to(pool, 0);
+       i_assert(pool->conn_list == NULL);
+       i_free(pool);
+}
+
+int ldap_connection_pool_get(struct ldap_connection_pool *pool,
+                            struct ldap_client *client,
+                            const struct ldap_client_settings *set,
+                            struct ldap_connection_list **list_r,
+                            const char **error_r)
+{
+       struct ldap_connection_list *list;
+       struct ldap_connection *conn;
+
+       for (list = pool->conn_list; list != NULL; list = list->next) {
+               if (ldap_connection_have_settings(list->conn, set)) {
+                       list->refcount++;
+                       *list_r = list;
+                       return 0;
+               }
+       }
+       if (ldap_connection_init(client, set, &conn, error_r) < 0)
+               return -1;
+
+       list = i_new(struct ldap_connection_list, 1);
+       list->conn = conn;
+       list->refcount++;
+
+       DLLIST_PREPEND(&pool->conn_list, list);
+       pool->conn_count++;
+
+       ldap_connection_pool_shrink_to(pool, pool->max_connections);
+       *list_r = list;
+       return 0;
+}
+
+void ldap_connection_pool_unref(struct ldap_connection_pool *pool,
+                               struct ldap_connection_list **_list)
+{
+       struct ldap_connection_list *list = *_list;
+
+       *_list = NULL;
+
+       i_assert(list->refcount > 0);
+
+       if (--list->refcount == 0)
+               ldap_connection_pool_shrink_to(pool, pool->max_connections);
+}
+
+bool ldap_connection_pool_have_references(struct ldap_connection_pool *pool)
+{
+       struct ldap_connection_list *list;
+
+       for (list = pool->conn_list; list != NULL; list = list->next) {
+               if (list->refcount > 0)
+                       return TRUE;
+       }
+       return FALSE;
+}

diff --git a/src/lib-ldap/ldap-connection-pool.h b/src/lib-ldap/ldap-connection-pool.h
new file mode 100644 (file)
index 0000000..00cf165
--- /dev/null
+++ b/src/lib-ldap/ldap-connection-pool.h
@@ -0,0 +1,27 @@
+#ifndef LDAP_CONNECTION_POOL_H
+#define LDAP_CONNECTION_POOL_H
+
+struct ldap_client;
+struct ldap_client_settings;
+
+struct ldap_connection_list {
+       struct ldap_connection_list *prev, *next;
+       struct ldap_connection *conn;
+       int refcount;
+};
+
+struct ldap_connection_pool *
+ldap_connection_pool_init(unsigned int max_connections);
+void ldap_connection_pool_deinit(struct ldap_connection_pool **_pool);
+/* Returns TRUE if there are connections with refcount>0 */
+bool ldap_connection_pool_have_references(struct ldap_connection_pool *pool);
+
+int ldap_connection_pool_get(struct ldap_connection_pool *pool,
+                            struct ldap_client *client,
+                            const struct ldap_client_settings *set,
+                            struct ldap_connection_list **list_r,
+                            const char **error_r);
+void ldap_connection_pool_unref(struct ldap_connection_pool *pool,
+                               struct ldap_connection_list **list);
+
+#endif

diff --git a/src/lib-ldap/ldap-connection.c b/src/lib-ldap/ldap-connection.c
index 8a468bde44de0cc2786b442e56b0e5ecd792faec..fada2cb361d326a721e7e319751dc254bc406722 100644 (file)
--- a/src/lib-ldap/ldap-connection.c
+++ b/src/lib-ldap/ldap-connection.c
@@ -88,6 +88,41 @@ int ldap_connection_setup(struct ldap_connection *conn, const char **error_r)
        return 0;
 }
 
+bool ldap_connection_have_settings(struct ldap_connection *conn,
+                                  const struct ldap_client_settings *set)
+{
+       const struct ldap_client_settings *conn_set = &conn->set;
+
+       if (strcmp(conn_set->uri, set->uri) != 0)
+               return FALSE;
+       if (null_strcmp(conn_set->bind_dn, set->bind_dn) != 0)
+               return FALSE;
+       if (null_strcmp(conn_set->password, set->password) != 0)
+               return FALSE;
+       if (conn_set->timeout_secs != set->timeout_secs ||
+           conn_set->max_idle_time_secs != set->max_idle_time_secs ||
+           conn_set->debug != set->debug ||
+           conn_set->require_ssl != set->require_ssl ||
+           conn_set->start_tls != set->start_tls)
+               return FALSE;
+
+       if (set->ssl_set == NULL || !set->start_tls)
+               return TRUE;
+
+       /* check SSL settings */
+       if (null_strcmp(conn->ssl_set.protocols, set->ssl_set->protocols) != 0)
+               return FALSE;
+       if (null_strcmp(conn->ssl_set.cipher_list, set->ssl_set->cipher_list) != 0)
+               return FALSE;
+       if (null_strcmp(conn->ssl_set.ca_file, set->ssl_set->ca_file) != 0)
+               return FALSE;
+       if (null_strcmp(conn->ssl_set.cert, set->ssl_set->cert) != 0)
+               return FALSE;
+       if (null_strcmp(conn->ssl_set.key, set->ssl_set->key) != 0)
+               return FALSE;
+       return TRUE;
+}
+
 int ldap_connection_init(struct ldap_client *client,
                         const struct ldap_client_settings *set,
                         struct ldap_connection **conn_r, const char **error_r)
@@ -122,12 +157,15 @@ int ldap_connection_init(struct ldap_client *client,
        conn->ssl_set.crypto_device = NULL;
 
        if (set->ssl_set != NULL) {
+               /* keep in sync with ldap_connection_have_settings() */
+               conn->set.ssl_set = &conn->ssl_set;
                conn->ssl_set.protocols = p_strdup(pool, set->ssl_set->protocols);
                conn->ssl_set.cipher_list = p_strdup(pool, set->ssl_set->cipher_list);
                conn->ssl_set.ca_file = p_strdup(pool, set->ssl_set->ca_file);
                conn->ssl_set.cert = p_strdup(pool, set->ssl_set->cert);
                conn->ssl_set.key = p_strdup(pool, set->ssl_set->key);
        }
+       i_assert(ldap_connection_have_settings(conn, set));
 
        if (ldap_connection_setup(conn, error_r) < 0) {
                ldap_connection_deinit(&conn);

diff --git a/src/lib-ldap/ldap-private.h b/src/lib-ldap/ldap-private.h
index e6004db8b23a243c6e9a3550ee634b20e2bc9373..fa724f40df966e985ac9dab3899e6aac4ce085f1 100644 (file)
--- a/src/lib-ldap/ldap-private.h
+++ b/src/lib-ldap/ldap-private.h
@@ -108,6 +108,8 @@ int ldap_connection_init(struct ldap_client *client,
                         struct ldap_connection **conn_r, const char **error_r);
 void ldap_connection_deinit(struct ldap_connection **_conn);
 void ldap_connection_switch_ioloop(struct ldap_connection *conn);
+bool ldap_connection_have_settings(struct ldap_connection *conn,
+                                  const struct ldap_client_settings *set);
 
 void ldap_connection_search_start(struct ldap_connection *conn,
                                  const struct ldap_search_input *input,