From 9652cc61bb130becf44c65a8b871f5badda237d6 Mon Sep 17 00:00:00 2001
From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Fri, 20 Jun 2014 22:18:51 +0100
Subject: [PATCH] Alloc connections pools in the NULL ctx

---
 src/include/libradius.h |  2 +-
 src/lib/debug.c         |  7 +-----
 src/lib/misc.c          | 55 +++++++++++++++++++++++++++++++----------
 src/main/connection.c   | 20 +++++++++++----
 4 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/src/include/libradius.h b/src/include/libradius.h
index 8c438da89be5..25a24c3ae72d 100644
--- a/src/include/libradius.h
+++ b/src/include/libradius.h
@@ -647,7 +647,7 @@ void		fr_printf_log(char const *, ...) CC_HINT(format (printf, 1, 2));
  *	Several handy miscellaneous functions.
  */
 int		fr_set_signal(int sig, sig_t func);
-TALLOC_CTX	*fr_autofree_ctx(void);
+int		fr_link_talloc_ctx_free(TALLOC_CTX *parent, TALLOC_CTX *child);
 char const	*fr_inet_ntop(int af, void const *src);
 char const 	*ip_ntoa(char *, uint32_t);
 int		fr_pton4(fr_ipaddr_t *out, char const *value, size_t inlen, bool resolve, bool fallback);
diff --git a/src/lib/debug.c b/src/lib/debug.c
index 7051a922187b..f0e0a20b3f19 100644
--- a/src/lib/debug.c
+++ b/src/lib/debug.c
@@ -215,12 +215,7 @@ fr_bt_marker_t *fr_backtrace_attach(fr_cbuff_t **cbuff, TALLOC_CTX *obj)
 	if (*cbuff == NULL) {
 		PTHREAD_MUTEX_LOCK(&fr_debug_init);
 		/* Check again now we hold the mutex - eww*/
-		if (*cbuff == NULL) {
-			TALLOC_CTX *ctx;
-
-			ctx = fr_autofree_ctx();
-			*cbuff = fr_cbuff_alloc(ctx, MAX_BT_CBUFF, true);
-		}
+		if (*cbuff == NULL) *cbuff = fr_cbuff_alloc(NULL, MAX_BT_CBUFF, true);
 		PTHREAD_MUTEX_UNLOCK(&fr_debug_init);
 	}
 
diff --git a/src/lib/misc.c b/src/lib/misc.c
index c9dc094904ee..d4d87f4241bc 100644
--- a/src/lib/misc.c
+++ b/src/lib/misc.c
@@ -35,7 +35,6 @@ RCSID("$Id$")
 	} while (0)
 
 #ifdef HAVE_PTHREAD_H
-static pthread_mutex_t autofree_context = PTHREAD_MUTEX_INITIALIZER;
 #  define PTHREAD_MUTEX_LOCK pthread_mutex_lock
 #  define PTHREAD_MUTEX_UNLOCK pthread_mutex_unlock
 #else
@@ -53,6 +52,11 @@ static char const *months[] = {
 
 fr_thread_local_setup(char *, fr_inet_ntop_buffer);	/* macro */
 
+typedef struct fr_talloc_link {
+	bool armed;
+	TALLOC_CTX *child;
+} fr_talloc_link_t;
+
 /** Sets a signal handler using sigaction if available, else signal
  *
  * @param sig to set handler for.
@@ -81,25 +85,50 @@ int fr_set_signal(int sig, sig_t func)
 	return 0;
 }
 
-/** Allocates a new talloc context from the root autofree context
+static int _fr_trigger_talloc_ctx_free(fr_talloc_link_t *trigger)
+{
+	if (trigger->armed) talloc_free(trigger->child);
+
+	return 0;
+}
+
+static int _fr_disarm_talloc_ctx_free(bool **armed)
+{
+	**armed = false;
+	return 0;
+}
+
+/** Link a parent and a child context, so the child is freed before the parent
  *
- * This function is threadsafe, whereas using the NULL context is not.
+ * @note This is not thread safe. Do not free parent before threads are joined, do not call from a child thread.
+ * @note It's OK to free the child before threads are joined, but this will leak memory until the parent is freed.
  *
- * @note The returned context must be freed by the caller.
- * @returns a new talloc context parented by the root autofree context.
+ * @param parent who's fate the child should share.
+ * @param child bound to parent's lifecycle.
+ * @return 0 on success -1 on failure.
  */
-TALLOC_CTX *fr_autofree_ctx(void)
+int fr_link_talloc_ctx_free(TALLOC_CTX *parent, TALLOC_CTX *child)
 {
-	static TALLOC_CTX *ctx = NULL, *child;
-	PTHREAD_MUTEX_LOCK(&autofree_context);
-	if (!ctx) {
-		ctx = talloc_autofree_context();
+	fr_talloc_link_t *trigger;
+	bool **disarm;
+
+	trigger = talloc(parent, fr_talloc_link_t);
+	if (!trigger) return -1;
+
+	disarm = talloc(child, bool *);
+	if (!disarm) {
+		talloc_free(trigger);
+		return -1;
 	}
 
-	child = talloc_new(ctx);
-	PTHREAD_MUTEX_UNLOCK(&autofree_context);
+	trigger->child = child;
+	trigger->armed = true;
+	*disarm = &trigger->armed;
+
+	talloc_set_destructor(trigger, _fr_trigger_talloc_ctx_free);
+	talloc_set_destructor(disarm, _fr_disarm_talloc_ctx_free);
 
-	return child;
+	return 0;
 }
 
 /*
diff --git a/src/main/connection.c b/src/main/connection.c
index 3c9e586dac85..1f14b0dce880 100644
--- a/src/main/connection.c
+++ b/src/main/connection.c
@@ -587,13 +587,23 @@ fr_connection_pool_t *fr_connection_pool_init(CONF_SECTION *parent,
 	cs = cf_section_sub_find(parent, "pool");
 	if (!cs) cs = cf_section_sub_find(parent, "limit");
 
-	if (cs) {
-		pool = talloc_zero(cs, fr_connection_pool_t);
-	} else {
-		pool = talloc_zero(parent, fr_connection_pool_t);
-	}
+	/*
+	 *	Pool is allocated in the NULL context as
+	 *	threads are likely to allocate memory
+	 *	beneath the pool.
+	 */
+	pool = talloc_zero(NULL, fr_connection_pool_t);
 	if (!pool) return NULL;
 
+	/*
+	 *	Ensure the pool is freed at the same time
+	 *	as its parent.
+	 */
+	if (fr_link_talloc_ctx_free(cs ? cs : parent, pool) < 0) {
+		talloc_free(pool);
+		return NULL;
+	}
+
 	pool->cs = cs;
 	pool->ctx = ctx;
 	pool->create = c;