Pass a threadsafe ctx into fr_connection_pool create callback
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Mon, 23 Jun 2014 13:54:13 +0000 (14:54 +0100)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Mon, 23 Jun 2014 15:19:54 +0000 (16:19 +0100)
Create callbacks should allocate any connection specific data in this specially created thread safe ctx.

For freeing connection specific data, a talloc destructor should be used. The delete callback will be
removed shortly.

18 files changed:
src/include/connection.h
src/main/connection.c
src/modules/rlm_couchbase/mod.c
src/modules/rlm_couchbase/mod.h
src/modules/rlm_couchbase/rlm_couchbase.c
src/modules/rlm_krb5/krb5.c
src/modules/rlm_krb5/krb5.h
src/modules/rlm_krb5/rlm_krb5.c
src/modules/rlm_ldap/ldap.c
src/modules/rlm_ldap/ldap.h
src/modules/rlm_ldap/rlm_ldap.c
src/modules/rlm_redis/rlm_redis.c
src/modules/rlm_rest/rest.c
src/modules/rlm_rest/rest.h
src/modules/rlm_rest/rlm_rest.c
src/modules/rlm_smsotp/rlm_smsotp.c
src/modules/rlm_sql/sql.c
src/modules/rlm_yubikey/validate.c

index 5e22116..8b7473a 100644 (file)
@@ -40,12 +40,22 @@ typedef struct fr_connection_pool_t fr_connection_pool_t;
  * This function will be called whenever the connection pool manager needs
  * to spawn a new connection, and on reconnect.
  *
+ * Memory should be talloced in the parent context to hold the module's
+ * connection structure. The parent context is allocated in the NULL
+ * context, but will be freed when fr_connection_t is freed.
+ *
+ * There is no delete callback, so operations such as closing sockets and
+ * freeing library connection handles should be done by a destructor attached
+ * to memory allocated beneath parent.
+ *
  * @note A function pointer matching this prototype must be passed
  * to fr_connection_pool.
- * @param[in] ctx pointer passed to fr_connection_pool_init.
+ *
+ * @param[in,out] ctx to allocate memory in.
+ * @param[in] opaque pointer passed to fr_connection_pool_init.
  * @return NULL on error, else a connection handle.
  */
-typedef void *(*fr_connection_create_t)(void *ctx);
+typedef void *(*fr_connection_create_t)(TALLOC_CTX *ctx, void *opaque);
 
 /** Check a connection handle is still viable
  *
@@ -54,25 +64,25 @@ typedef void *(*fr_connection_create_t)(void *ctx);
  * @note NULL may be passed to fr_connection_init, if there is no way to check
  * the state of a connection handle.
  * @note Not currently use by connection pool manager.
- * @param[in] ctx pointer passed to fr_connection_pool_init.
+ * @param[in] opaque pointer passed to fr_connection_pool_init.
  * @param[in] connection handle returned by fr_connection_create_t.
  * @return < 0 on error or if the connection is unusable, else 0.
  */
-typedef int (*fr_connection_alive_t)(void *ctx, void *connection);
+typedef int (*fr_connection_alive_t)(void *opaque, void *connection);
 
 /** Delete a connection and free allocated memory
  *
  * Should close any sockets associated with the passed connection handle,
  * and free any memory allocated to it.
  *
- * @param[in] ctx pointer passed to fr_connection_pool_init.
+ * @param[in] opaque pointer passed to fr_connection_pool_init.
  * @param[in,out] connection handle returned by fr_connection_create_t.
  * @return < 0 on error else 0 if connection was closed successfully.
  */
-typedef int (*fr_connection_delete_t)(void *ctx, void *connection);
+typedef int (*fr_connection_delete_t)(void *opaque, void *connection);
 
 fr_connection_pool_t *fr_connection_pool_init(CONF_SECTION *cs,
-                                             void *ctx,
+                                             void *opaque,
                                              fr_connection_create_t c,
                                              fr_connection_alive_t a,
                                              fr_connection_delete_t d,
index 1f14b0d..ff0db71 100644 (file)
@@ -151,7 +151,7 @@ struct fr_connection_pool_t {
        CONF_SECTION    *cs;            //!< Configuration section holding
                                        //!< the section of parsed config file
                                        //!< that relates to this pool.
-       void            *ctx;           //!< Pointer to context data that will
+       void            *opaque;        //!< Pointer to context data that will
                                        //!< be passed to callbacks.
 
        char            *log_prefix;    //!< Log prefix to prepend to all log
@@ -291,6 +291,8 @@ static void fr_connection_link_tail(fr_connection_pool_t *pool,
 static fr_connection_t *fr_connection_spawn(fr_connection_pool_t *pool,
                                            time_t now, bool in_use)
 {
+       TALLOC_CTX *ctx;
+
        fr_connection_t *this;
        void *conn;
 
@@ -356,12 +358,19 @@ static fr_connection_t *fr_connection_spawn(fr_connection_pool_t *pool,
        INFO("%s: Opening additional connection (%" PRIu64 ")", pool->log_prefix, pool->count);
 
        /*
+        *      Allocate a new top level ctx for the create callback
+        *      to hang its memory off of.
+        */
+       ctx = talloc_init("fr_connection_ctx");
+       if (!ctx) return NULL;
+
+       /*
         *      This may take a long time, which prevents other
         *      threads from releasing connections.  We don't care
         *      about other threads opening new connections, as we
         *      already have no free connections.
         */
-       conn = pool->create(pool->ctx);
+       conn = pool->create(ctx, pool->opaque);
        if (!conn) {
                ERROR("%s: Opening connection failed (%" PRIu64 ")", pool->log_prefix, pool->count);
 
@@ -381,6 +390,7 @@ static fr_connection_t *fr_connection_spawn(fr_connection_pool_t *pool,
                pthread_mutex_unlock(&pool->mutex);
                return NULL;
        }
+       fr_link_talloc_ctx_free(this, ctx);
 
        this->created = now;
        this->connection = conn;
@@ -435,7 +445,7 @@ static void fr_connection_close(fr_connection_pool_t *pool,
        if (pool->trigger) exec_trigger(NULL, pool->cs, "close", true);
 
        fr_connection_unlink(pool, this);
-       pool->delete(pool->ctx, this->connection);
+       if (pool->delete) pool->delete(pool->opaque, this->connection);
        rad_assert(pool->num > 0);
        pool->num--;
        talloc_free(this);
@@ -559,7 +569,7 @@ void fr_connection_pool_delete(fr_connection_pool_t *pool)
  * @note Will call the 'start' trigger.
  *
  * @param[in] parent configuration section containing a 'pool' subsection.
- * @param[in] ctx pointer to pass to callbacks.
+ * @param[in] opaque data pointer to pass to callbacks.
  * @param[in] c Callback to create new connections.
  * @param[in] a Callback to check the status of connections.
  * @param[in] d Callback to delete connections.
@@ -568,7 +578,7 @@ void fr_connection_pool_delete(fr_connection_pool_t *pool)
  * @return A new connection pool or NULL on error.
  */
 fr_connection_pool_t *fr_connection_pool_init(CONF_SECTION *parent,
-                                             void *ctx,
+                                             void *opaque,
                                              fr_connection_create_t c,
                                              fr_connection_alive_t a,
                                              fr_connection_delete_t d,
@@ -582,7 +592,7 @@ fr_connection_pool_t *fr_connection_pool_init(CONF_SECTION *parent,
        char const *cs_name1, *cs_name2;
        time_t now = time(NULL);
 
-       if (!parent || !ctx || !c || !d) return NULL;
+       if (!parent || !opaque || !c) return NULL;
 
        cs = cf_section_sub_find(parent, "pool");
        if (!cs) cs = cf_section_sub_find(parent, "limit");
@@ -605,7 +615,7 @@ fr_connection_pool_t *fr_connection_pool_init(CONF_SECTION *parent,
        }
 
        pool->cs = cs;
-       pool->ctx = ctx;
+       pool->opaque = opaque;
        pool->create = c;
        pool->alive = a;
        pool->delete = d;
@@ -1094,20 +1104,35 @@ void *fr_connection_reconnect(fr_connection_pool_t *pool, void *conn)
        void *new_conn;
        fr_connection_t *this;
        uint64_t conn_number;
+       TALLOC_CTX *ctx;
 
        if (!pool || !conn) return NULL;
 
+       /*
+        *      If fr_connection_find is successful the pool is now locked
+        */
        this = fr_connection_find(pool, conn);
        if (!this) return NULL;
 
+
+       conn_number = this->number;
+
        /*
-        *      The pool is now locked.
+        *      Destroy any handles associated with the fr_connection_t
         */
-       conn_number = this->number;
+       talloc_free_children(this);
 
        DEBUG("%s: Reconnecting (%" PRIu64 ")", pool->log_prefix, conn_number);
 
-       new_conn = pool->create(pool->ctx);
+       /*
+        *      Allocate a new top level ctx for the create callback
+        *      to hang its memory off of.
+        */
+       ctx = talloc_init("fr_connection_ctx");
+       if (!ctx) return NULL;
+       fr_link_talloc_ctx_free(this, ctx);
+
+       new_conn = pool->create(ctx, pool->opaque);
        if (!new_conn) {
                /*
                 *      We can't create a new connection, so close
@@ -1123,13 +1148,14 @@ void *fr_connection_reconnect(fr_connection_pool_t *pool, void *conn)
                new_conn = fr_connection_get_internal(pool, false);
                if (new_conn) return new_conn;
 
-               RATE_LIMIT(ERROR("%s: Failed to reconnect (%" PRIu64 "), no free connections are available", pool->log_prefix,
-                                conn_number));
+               RATE_LIMIT(ERROR("%s: Failed to reconnect (%" PRIu64 "), no free connections are available",
+                                pool->log_prefix, conn_number));
+
                return NULL;
        }
 
        if (pool->trigger) exec_trigger(NULL, pool->cs, "close", true);
-       pool->delete(pool->ctx, conn);
+       pool->delete(pool->opaque, conn);
        this->connection = new_conn;
        pthread_mutex_unlock(&pool->mutex);
        return new_conn;
index a7a163c..5f9d606 100644 (file)
@@ -34,8 +34,20 @@ RCSID("$Id$");
 #include "couchbase.h"
 #include "jsonc_missing.h"
 
+/* free couchbase instance handle and any additional context memory */
+static int _mod_conn_free(rlm_couchbase_handle_t *chandle)
+{
+       lcb_t cb_inst = chandle->handle;                /* couchbase instance */
+
+       /* destroy/free couchbase instance */
+       lcb_destroy(cb_inst);
+
+       /* return */
+       return 0;
+}
+
 /* create new connection pool handle */
-void *mod_conn_create(void *instance)
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
        rlm_couchbase_t *inst = instance;           /* module instance pointer */
        rlm_couchbase_handle_t *chandle = NULL;     /* connection handle pointer */
@@ -56,7 +68,9 @@ void *mod_conn_create(void *instance)
        }
 
        /* allocate memory for couchbase connection instance abstraction */
-       chandle = talloc_zero(inst, rlm_couchbase_handle_t);
+       chandle = talloc_zero(ctx, rlm_couchbase_handle_t);
+       talloc_set_destructor(chandle, _mod_conn_free);
+
        cookie = talloc_zero(chandle, cookie_t);
 
        /* initialize cookie error holder */
@@ -90,22 +104,6 @@ int mod_conn_alive(UNUSED void *instance, void *handle)
        return true;
 }
 
-/* free couchbase instance handle and any additional context memory */
-int mod_conn_delete(UNUSED void *instance, void *handle)
-{
-       rlm_couchbase_handle_t *chandle = handle;       /* connection instance handle */
-       lcb_t cb_inst = chandle->handle;                /* couchbase instance */
-
-       /* destroy/free couchbase instance */
-       lcb_destroy(cb_inst);
-
-       /* free handle */
-       talloc_free(chandle);
-
-       /* return */
-       return true;
-}
-
 /* build json object for mapping radius attributes to json elements */
 int mod_build_attribute_element_map(CONF_SECTION *conf, void *instance)
 {
index 3789f4f..8c475ee 100644 (file)
@@ -62,12 +62,10 @@ typedef struct rlm_couchbase_handle_t {
 } rlm_couchbase_handle_t;
 
 /* define functions */
-void *mod_conn_create(void *instance);
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance);
 
 int mod_conn_alive(UNUSED void *instance, void *handle);
 
-int mod_conn_delete(UNUSED void *instance, void *handle);
-
 int mod_build_attribute_element_map(CONF_SECTION *conf, void *instance);
 
 int mod_attribute_to_element(const char *name, json_object *map, void *buf);
index ed1bbb9..854ecf4 100644 (file)
@@ -100,7 +100,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance) {
        }
 
        /* initiate connection pool */
-       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, mod_conn_alive, mod_conn_delete, NULL);
+       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, mod_conn_alive, NULL, NULL);
 
        /* check connection pool */
        if (!inst->pool) {
index 33123ca..af96238 100644 (file)
@@ -89,17 +89,6 @@ char const *rlm_krb5_error(krb5_context context, krb5_error_code code)
 }
 #endif
 
-/** Frees a krb5 context
- *
- * @param instance rlm_krb5 instance.
- * @param handle to destroy.
- * @return 0 (always indicates success).
- */
-int mod_conn_delete(UNUSED void *instance, void *handle)
-{
-       return talloc_free((krb5_context *) handle);
-}
-
 /** Frees libkrb5 resources associated with the handle
  *
  * Must not be called directly.
@@ -107,7 +96,7 @@ int mod_conn_delete(UNUSED void *instance, void *handle)
  * @param conn to free.
  * @return 0 (always indicates success).
  */
-static int _free_handle(rlm_krb5_handle_t *conn) {
+static int _mod_conn_free(rlm_krb5_handle_t *conn) {
        krb5_free_context(conn->context);
 
        if (conn->keytab) {
@@ -129,16 +118,17 @@ static int _free_handle(rlm_krb5_handle_t *conn) {
  * by libkrb5 and that it does connection caching associated with contexts, so it's
  * worth using a connection pool to preserve connections when workers die.
  *
+ * @param ctx to allocate connection handle memory in.
  * @param instance rlm_krb5 instance instance.
  * @return A new context or NULL on error.
  */
-void *mod_conn_create(void *instance)
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
        rlm_krb5_t *inst = instance;
        rlm_krb5_handle_t *conn;
        krb5_error_code ret;
 
-       MEM(conn = talloc_zero(instance, rlm_krb5_handle_t));
+       MEM(conn = talloc_zero(ctx, rlm_krb5_handle_t));
        ret = krb5_init_context(&conn->context);
        if (ret) {
                ERROR("rlm_krb5 (%s): Context initialisation failed: %s", inst->xlat_name,
@@ -146,7 +136,7 @@ void *mod_conn_create(void *instance)
 
                return NULL;
        }
-       talloc_set_destructor(conn, _free_handle);
+       talloc_set_destructor(conn, _mod_conn_free);
 
        ret = inst->keytabname ?
                krb5_kt_resolve(conn->context, inst->keytabname, &conn->keytab) :
index 59b1f85..0d2d584 100644 (file)
@@ -90,6 +90,4 @@ typedef struct rlm_krb5_t {
 char const *rlm_krb5_error(krb5_context context, krb5_error_code code);
 #endif
 
-void *mod_conn_create(void *instance);
-
-int mod_conn_delete(UNUSED void *instance, void *handle);
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance);
index adc5e3e..14743d5 100644 (file)
@@ -222,12 +222,12 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
        /*
         *      Initialize the socket pool.
         */
-       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, mod_conn_delete, NULL);
+       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, NULL, NULL);
        if (!inst->pool) {
                return -1;
        }
 #else
-       inst->conn = mod_conn_create(inst);
+       inst->conn = mod_conn_create(inst, inst);
        if (!inst->conn) {
                return -1;
        }
index 70d2813..6fe2473 100644 (file)
@@ -1088,14 +1088,32 @@ static int rlm_ldap_rebind(LDAP *handle, LDAP_CONST char *url, UNUSED ber_tag_t
 }
 #endif
 
+/** Close and delete a connection
+ *
+ * Unbinds the LDAP connection, informing the server and freeing any memory, then releases the memory used by the
+ * connection handle.
+ *
+ * @param conn to destroy.
+ * @return always indicates success.
+ */
+static int _mod_conn_free(ldap_handle_t *conn)
+{
+       DEBUG3("rlm_ldap: Closing libldap handle %p", conn->handle);
+
+       if (conn->handle) ldap_unbind_s(conn->handle);
+
+       return 0;
+}
+
 /** Create and return a new connection
  *
  * Create a new ldap connection and allocate memory for a new rlm_handle_t
  *
+ * @param ctx to allocate connection handle memory in.
  * @param instance rlm_ldap instance.
  * @return A new connection handle or NULL on error.
  */
-void *mod_conn_create(void *instance)
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
        ldap_rcode_t status;
 
@@ -1108,8 +1126,9 @@ void *mod_conn_create(void *instance)
        /*
         *      Allocate memory for the handle.
         */
-       conn = talloc_zero(instance, ldap_handle_t);
+       conn = talloc_zero(ctx, ldap_handle_t);
        if (!conn) return NULL;
+       talloc_set_destructor(conn, _mod_conn_free);
 
        conn->inst = inst;
        conn->rebound = false;
@@ -1135,6 +1154,7 @@ void *mod_conn_create(void *instance)
                        goto error;
                }
        }
+       DEBUG3("rlm_ldap: New libldap handle %p", conn->handle);
 
        /*
         *      We now have a connection structure, but no actual TCP connection.
@@ -1275,31 +1295,11 @@ void *mod_conn_create(void *instance)
        return conn;
 
 error:
-       if (conn->handle) ldap_unbind_s(conn->handle);
        talloc_free(conn);
 
        return NULL;
 }
 
-/** Close and delete a connection
- *
- * Unbinds the LDAP connection, informing the server and freeing any memory, then releases the memory used by the
- * connection handle.
- *
- * @param instance rlm_ldap instance.
- * @param handle to destroy.
- * @return always indicates success.
- */
-int mod_conn_delete(UNUSED void *instance, void *handle)
-{
-       ldap_handle_t *conn = handle;
-
-       ldap_unbind_s(conn->handle);
-       talloc_free(conn);
-
-       return 0;
-}
-
 /** Gets an LDAP socket from the connection pool
  *
  * Retrieve a socket from the connection pool, or NULL on error (of if no sockets are available).
index 0dac3f5..50bcb87 100644 (file)
@@ -346,9 +346,7 @@ void rlm_ldap_check_reply(ldap_instance_t const *inst, REQUEST *request);
 /*
  *     ldap.c - Callbacks for the connection pool API.
  */
-void *mod_conn_create(void *ctx);
-
-int mod_conn_delete(UNUSED void *instance, void *handle);
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance);
 
 ldap_handle_t *rlm_ldap_get_socket(ldap_instance_t const *inst, REQUEST *request);
 
index a6b7f09..e4697fd 100644 (file)
@@ -769,7 +769,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
        /*
         *      Initialize the socket pool.
         */
-       inst->pool = fr_connection_pool_init(inst->cs, inst, mod_conn_create, NULL, mod_conn_delete, NULL);
+       inst->pool = fr_connection_pool_init(inst->cs, inst, mod_conn_create, NULL, NULL, NULL);
        if (!inst->pool) {
                return -1;
        }
index f752719..f07d494 100644 (file)
@@ -39,10 +39,8 @@ static const CONF_PARSER module_config[] = {
        { NULL, -1, 0, NULL, NULL} /* end the list */
 };
 
-static int mod_conn_delete(UNUSED void *instance, void *handle)
+static int _mod_conn_free(REDISSOCK *dissocket)
 {
-       REDISSOCK *dissocket = handle;
-
        redisFree(dissocket->conn);
 
        if (dissocket->reply) {
@@ -50,13 +48,12 @@ static int mod_conn_delete(UNUSED void *instance, void *handle)
                dissocket->reply = NULL;
        }
 
-       talloc_free(dissocket);
-       return 1;
+       return 0;
 }
 
-static void *mod_conn_create(void *ctx)
+static void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
-       REDIS_INST *inst = ctx;
+       REDIS_INST *inst = instance;
        REDISSOCK *dissocket = NULL;
        redisContext *conn;
        redisReply *reply = NULL;
@@ -70,8 +67,8 @@ static void *mod_conn_create(void *ctx)
 
                reply = redisCommand(conn, buffer);
                if (!reply) {
-                       ERROR("rlm_redis (%s): Failed to run AUTH",
-                              inst->xlat_name);
+                       ERROR("rlm_redis (%s): Failed to run AUTH", inst->xlat_name);
+
                do_close:
                        if (reply) freeReplyObject(reply);
                        redisFree(conn);
@@ -123,8 +120,9 @@ static void *mod_conn_create(void *ctx)
                }
        }
 
-       dissocket = talloc_zero(inst, REDISSOCK);
+       dissocket = talloc_zero(ctx, REDISSOCK);
        dissocket->conn = conn;
+       talloc_set_destructor(dissocket, _mod_conn_free);
 
        return dissocket;
 }
@@ -278,7 +276,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
 
        xlat_register(inst->xlat_name, redis_xlat, NULL, inst); /* FIXME! */
 
-       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, mod_conn_delete, NULL);
+       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, NULL, NULL);
        if (!inst->pool) {
                return -1;
        }
index 8f6ce5f..27db0f5 100644 (file)
@@ -287,6 +287,19 @@ void rest_cleanup(void)
        curl_global_cleanup();
 }
 
+
+/** Frees a libcurl handle, and any additional memory used by context data.
+ *
+ * @param[in] randle rlm_rest_handle_t to close and free.
+ * @return returns true.
+ */
+static int _mod_conn_free(rlm_rest_handle_t *randle)
+{
+       curl_easy_cleanup(randle->handle);
+
+       return 0;
+}
+
 /** Creates a new connection handle for use by the FR connection API.
  *
  * Matches the fr_connection_create_t function prototype, is passed to
@@ -295,14 +308,13 @@ void rest_cleanup(void)
  *
  * Creates an instances of rlm_rest_handle_t, and rlm_rest_curl_context_t
  * which hold the context data required for generating requests and parsing
- * responses. Calling mod_conn_delete will free this memory.
+ * responses.
  *
  * If instance->connect_uri is not NULL libcurl will attempt to open a
  * TCP socket to the server specified in the URI. This is done so that when the
  * socket is first used, there will already be a cached TCP connection to the
  * REST server associated with the curl handle.
  *
- * @see mod_conn_delete
  * @see fr_connection_pool_init
  * @see fr_connection_create_t
  * @see connection.c
@@ -311,12 +323,12 @@ void rest_cleanup(void)
  * @return connection handle or NULL if the connection failed or couldn't
  *     be initialised.
  */
-void *mod_conn_create(void *instance)
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
        rlm_rest_t *inst = instance;
 
        rlm_rest_handle_t       *randle = NULL;
-       rlm_rest_curl_context_t *ctx = NULL;
+       rlm_rest_curl_context_t *curl_ctx = NULL;
 
        CURL *candle = curl_easy_init();
 
@@ -352,14 +364,15 @@ void *mod_conn_create(void *instance)
        /*
         *  Allocate memory for the connection handle abstraction.
         */
-       randle = talloc_zero(inst, rlm_rest_handle_t);
-       ctx = talloc_zero(randle, rlm_rest_curl_context_t);
+       randle = talloc_zero(ctx, rlm_rest_handle_t);
+       curl_ctx = talloc_zero(randle, rlm_rest_curl_context_t);
 
-       ctx->headers = NULL; /* CURL needs this to be NULL */
-       ctx->request.instance = inst;
+       curl_ctx->headers = NULL; /* CURL needs this to be NULL */
+       curl_ctx->request.instance = inst;
 
-       randle->ctx = ctx;
+       randle->ctx = curl_ctx;
        randle->handle = candle;
+       talloc_set_destructor(randle, _mod_conn_free);
 
        /*
         *  Clear any previously configured options for the first request.
@@ -419,24 +432,6 @@ int mod_conn_alive(void *instance, void *handle)
        return true;
 }
 
-/** Frees a libcurl handle, and any additional memory used by context data.
- *
- * @param[in] instance configuration data.
- * @param[in] handle rlm_rest_handle_t to close and free.
- * @return returns true.
- */
-int mod_conn_delete(UNUSED void *instance, void *handle)
-{
-       rlm_rest_handle_t       *randle = handle;
-       CURL                    *candle = randle->handle;
-
-       curl_easy_cleanup(candle);
-
-       talloc_free(randle);
-
-       return true;
-}
-
 /** Copies a pre-expanded xlat string to the output buffer
  *
  * @param[out] out Char buffer to write encoded data to.
index 796c3eb..d6f474f 100644 (file)
@@ -248,12 +248,10 @@ int rest_init(rlm_rest_t *instance);
 
 void rest_cleanup(void);
 
-void *mod_conn_create(void *instance);
+void *mod_conn_create(TALLOC_CTX *ctx, void *instance);
 
 int mod_conn_alive(void *instance, void *handle);
 
-int mod_conn_delete(void *instance, void *handle);
-
 /*
  *     Request processing API
  */
index 9c5fce6..b8a4df8 100644 (file)
@@ -670,7 +670,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
                return -1;
        }
 
-       inst->conn_pool = fr_connection_pool_init(conf, inst, mod_conn_create, mod_conn_alive, mod_conn_delete, NULL);
+       inst->conn_pool = fr_connection_pool_init(conf, inst, mod_conn_create, mod_conn_alive, NULL, NULL);
        if (!inst->conn_pool) {
                return -1;
        }
index e47cefe..4aecd19 100644 (file)
@@ -42,8 +42,13 @@ static const CONF_PARSER module_config[] = {
        { NULL, -1, 0, NULL, NULL }             /* end the list */
 };
 
+static int _mod_conn_free(int *fdp)
+{
+       close(*fdp);
+       return 0;
+}
 
-static void *mod_conn_create(void *instance)
+static void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
        int fd;
        struct sockaddr_un sa;
@@ -67,22 +72,13 @@ static void *mod_conn_create(void *instance)
                return NULL;
        }
 
-       fdp = talloc_zero(instance, int);
+       fdp = talloc_zero(ctx, int);
+       talloc_set_destructor(fdp, _mod_conn_free);
        *fdp = fd;
 
        return fdp;
 }
 
-static int mod_conn_delete(UNUSED void *instance, void *handle)
-{
-       int *fdp = handle;
-
-       close(*fdp);
-       talloc_free(fdp);
-       return 0;
-}
-
-
 /*
  * Full read with logging, and close on failure.
  * Returns nread on success, 0 on EOF, -1 on other failures.
@@ -176,7 +172,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
        /*
         *      Initialize the socket pool.
         */
-       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, mod_conn_delete, NULL);
+       inst->pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, NULL, NULL);
        if (!inst->pool) {
                return -1;
        }
index c4c0c02..962f49d 100644 (file)
@@ -39,7 +39,7 @@ RCSID("$Id$")
 #ifdef HAVE_PTHREAD_H
 #endif
 
-static int _sql_conn_destructor(rlm_sql_handle_t *conn)
+static int _sql_conn_free(rlm_sql_handle_t *conn)
 {
        rlm_sql_t *inst = conn->inst;
 
@@ -50,13 +50,18 @@ static int _sql_conn_destructor(rlm_sql_handle_t *conn)
        return 0;
 }
 
-static void *mod_conn_create(void *instance)
+static void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
        int rcode;
        rlm_sql_t *inst = instance;
        rlm_sql_handle_t *handle;
 
-       handle = talloc_zero(instance, rlm_sql_handle_t);
+       /*
+        *      Connections cannot be alloced from the inst or
+        *      pool contexts due to threading issues.
+        */
+       handle = talloc_zero(ctx, rlm_sql_handle_t);
+       if (!handle) return NULL;
 
        /*
         *      Handle requires a pointer to the SQL inst so the
@@ -70,7 +75,7 @@ static void *mod_conn_create(void *instance)
         *      Then we call our destructor to trigger an modules.sql.close
         *      event, then all the memory is freed.
         */
-       talloc_set_destructor(handle, _sql_conn_destructor);
+       talloc_set_destructor(handle, _sql_conn_free);
 
        rcode = (inst->module->sql_socket_init)(handle, inst->config);
        if (rcode != 0) {
@@ -95,14 +100,6 @@ static void *mod_conn_create(void *instance)
        return handle;
 }
 
-/*
- *     @todo Calls to this should eventually go away.
- */
-static int mod_conn_delete(UNUSED void *instance, void *handle)
-{
-       return talloc_free(handle);
-}
-
 /*************************************************************************
  *
  *     Function: sql_socket_pool_init
@@ -112,9 +109,7 @@ static int mod_conn_delete(UNUSED void *instance, void *handle)
  *************************************************************************/
 int sql_socket_pool_init(rlm_sql_t * inst)
 {
-       inst->pool = fr_connection_pool_init(inst->cs, inst,
-                                            mod_conn_create, NULL, mod_conn_delete,
-                                            NULL);
+       inst->pool = fr_connection_pool_init(inst->cs, inst, mod_conn_create, NULL, NULL, NULL);
        if (!inst->pool) return -1;
 
        return 1;
index 814494e..b6fd613 100644 (file)
 
 /** Frees a ykclient handle
  *
- * @param[in] instance configuration data.
- * @param[in] handle rlm_yubikey_handle_t to close and free.
- * @return returns true.
+ * @param[in] yandle rlm_yubikey_handle_t to close and free.
+ * @return returns 0.
  */
-static int mod_conn_delete(UNUSED void *instance, void *handle)
+static int _mod_conn_free(ykclient_handle_t **yandle)
 {
-       ykclient_handle_t *yandle = handle;
-
-       ykclient_handle_done(&yandle);
+       ykclient_handle_done(yandle);
 
-       return true;
+       return 0;
 }
 
 /** Creates a new connection handle for use by the FR connection API.
@@ -38,15 +35,16 @@ static int mod_conn_delete(UNUSED void *instance, void *handle)
  * @see fr_connection_create_t
  * @see connection.c
  *
+ * @param[in] ctx to allocate connection data from.
  * @param[in] instance configuration data.
  * @return connection handle or NULL if the connection failed or couldn't
  *     be initialised.
  */
-static void *mod_conn_create(void *instance)
+static void *mod_conn_create(TALLOC_CTX *ctx, void *instance)
 {
        rlm_yubikey_t *inst = instance;
        ykclient_rc status;
-       ykclient_handle_t *yandle;
+       ykclient_handle_t *yandle, **marker;
 
        status = ykclient_handle_init(inst->ykc, &yandle);
        if (status != YKCLIENT_OK) {
@@ -54,6 +52,9 @@ static void *mod_conn_create(void *instance)
 
                return NULL;
        }
+       marker = talloc(ctx, ykclient_handle_t *);
+       talloc_set_destructor(marker, _mod_conn_free);
+       *marker = yandle;
 
        return yandle;
 }
@@ -137,7 +138,7 @@ init:
        }
 
        snprintf(prefix, sizeof(prefix), "rlm_yubikey (%s)", inst->name);
-       inst->conn_pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, mod_conn_delete, prefix);
+       inst->conn_pool = fr_connection_pool_init(conf, inst, mod_conn_create, NULL, NULL, prefix);
        if (!inst->conn_pool) {
                ykclient_done(&inst->ykc);