Don't leak worker heaps

diff --git a/src/include/heap.h b/src/include/heap.h
index eca2bf95a5d48bbb40fdb43ab6513fbbd70c0337..3ccde24bd515b08b3915ca78019dc1f2333866ac 100644 (file)
--- a/src/include/heap.h
+++ b/src/include/heap.h
@@ -38,15 +38,17 @@ typedef struct fr_heap_t fr_heap_t;
 
 /** Creates a heap that can be used with non-talloced elements
  *
+ * @param[in] _ctx             Talloc ctx to allocate heap in.
  * @param[in] _cmp             Comparator used to compare elements.
  * @param[in] _type            Of elements.
  * @param[in] _field           to store heap indexes in.
  */
-#define fr_heap_create(_cmp, _type, _field) \
-       _fr_heap_create(_cmp, NULL, (size_t)offsetof(_type, _field))
+#define fr_heap_create(_ctx, _cmp, _type, _field) \
+       _fr_heap_create(_ctx, _cmp, NULL, (size_t)offsetof(_type, _field))
 
 /** Creates a heap that verifies elements are of a specific talloc type
  *
+ * @param[in] _ctx             Talloc ctx to allocate heap in.
  * @param[in] _cmp             Comparator used to compare elements.
  * @param[in] _talloc_type     of elements.
  * @param[in] _field           to store heap indexes in.
@@ -54,10 +56,10 @@ typedef struct fr_heap_t fr_heap_t;
  *     - A new heap.
  *     - NULL on error.
  */
-#define fr_heap_talloc_create(_cmp, _talloc_type, _field) \
-       _fr_heap_create(_cmp, #_talloc_type, (size_t)offsetof(_talloc_type, _field))
+#define fr_heap_talloc_create(_ctx, _cmp, _talloc_type, _field) \
+       _fr_heap_create(_ctx, _cmp, #_talloc_type, (size_t)offsetof(_talloc_type, _field))
 
-fr_heap_t      *_fr_heap_create(fr_heap_cmp_t cmp, char const *talloc_type, size_t offset);
+fr_heap_t      *_fr_heap_create(TALLOC_CTX *ctx, fr_heap_cmp_t cmp, char const *talloc_type, size_t offset);
 
 int            fr_heap_insert(fr_heap_t *hp, void *data);
 int            fr_heap_extract(fr_heap_t *hp, void *data);

diff --git a/src/lib/io/network.c b/src/lib/io/network.c
index 2c726c65d4c541e104548d014daad48808d9d0fb..1639fd300bad6b6dab4ab5ef915ed282067ed548 100644 (file)
--- a/src/lib/io/network.c
+++ b/src/lib/io/network.c
@@ -699,7 +699,7 @@ static void fr_network_socket_callback(void *ctx, void const *data, size_t data_
        rad_assert(s != NULL);
        memcpy(&s->listen, data, sizeof(s->listen));
 
-       MEM(s->waiting = fr_heap_create(waiting_cmp, fr_channel_data_t, channel.heap_id));
+       MEM(s->waiting = fr_heap_create(s, waiting_cmp, fr_channel_data_t, channel.heap_id));
        FR_DLIST_INIT(s->entry);
 
        talloc_set_destructor(s, _network_socket_free);
@@ -773,7 +773,7 @@ static void fr_network_directory_callback(void *ctx, void const *data, size_t da
        rad_assert(s != NULL);
        memcpy(&s->listen, data, sizeof(s->listen));
 
-       MEM(s->waiting = fr_heap_create(waiting_cmp, fr_channel_data_t, channel.heap_id));
+       MEM(s->waiting = fr_heap_create(s, waiting_cmp, fr_channel_data_t, channel.heap_id));
        FR_DLIST_INIT(s->entry);
 
        talloc_set_destructor(s, _network_socket_free);
@@ -1017,7 +1017,7 @@ fr_network_t *fr_network_create(TALLOC_CTX *ctx, fr_event_list_t *el, fr_log_t c
                goto fail2;
        }
 
-       nr->replies = fr_heap_create(reply_cmp, fr_channel_data_t, channel.heap_id);
+       nr->replies = fr_heap_create(nr, reply_cmp, fr_channel_data_t, channel.heap_id);
        if (!nr->replies) {
                fr_strerror_printf_push("Failed creating heap for replies");
                goto fail2;

diff --git a/src/lib/io/worker.c b/src/lib/io/worker.c
index 545fd958530a2e1b5169d13470ab50a55253c7af..08e7ef39f6a0b47b6afae53d65166a00fbb5802c 100644 (file)
--- a/src/lib/io/worker.c
+++ b/src/lib/io/worker.c
@@ -163,7 +163,7 @@ static void fr_worker_post_event(fr_event_list_t *el, struct timeval *now, void
  */
 #define WORKER_HEAP_INIT(_name, _func, _type, _member) do { \
                FR_DLIST_INIT(worker->_name.list); \
-               worker->_name.heap = fr_heap_create(_func, _type, _member); \
+               worker->_name.heap = fr_heap_create(worker, _func, _type, _member); \
                if (!worker->_name.heap) { \
                        (void) fr_event_user_delete(worker->el, fr_worker_evfilt_user, worker); \
                        talloc_free(worker); \
@@ -1217,10 +1217,7 @@ void fr_worker_destroy(fr_worker_t *worker)
                worker_stop_request(worker, request, now);
                talloc_free(request);
        }
-       talloc_free(worker->time_order);
-
        rad_assert(fr_heap_num_elements(worker->runnable) == 0);
-       talloc_free(worker->runnable);
 
 #if 0
        /*
@@ -1336,13 +1333,13 @@ nomem:
        WORKER_HEAP_INIT(to_decode, worker_message_cmp, fr_channel_data_t, channel.heap_id);
        WORKER_HEAP_INIT(localized, worker_message_cmp, fr_channel_data_t, channel.heap_id);
 
-       worker->runnable = fr_heap_talloc_create(worker_runnable_cmp, REQUEST, runnable_id);
+       worker->runnable = fr_heap_talloc_create(worker, worker_runnable_cmp, REQUEST, runnable_id);
        if (!worker->runnable) {
                fr_strerror_printf("Failed creating runnable heap");
                goto fail;
        }
 
-       worker->time_order = fr_heap_talloc_create(worker_time_order_cmp, REQUEST, time_order_id);
+       worker->time_order = fr_heap_talloc_create(worker, worker_time_order_cmp, REQUEST, time_order_id);
        if (!worker->time_order) {
                fr_strerror_printf("Failed creating time_order heap");
                goto fail;

diff --git a/src/lib/util/event.c b/src/lib/util/event.c
index 6b81ec99301e420c121d519590dcde30c3044885..0f4eaa718548fcbbcd1f353cb8bf20e345734281 100644 (file)
--- a/src/lib/util/event.c
+++ b/src/lib/util/event.c
@@ -1786,8 +1786,6 @@ static int _event_list_free(fr_event_list_t *el)
 
        while ((ev = fr_heap_peek(el->times)) != NULL) fr_event_timer_delete(el, &ev);
 
-       talloc_free(el->times);
-
        talloc_free_children(el);
 
        if (el->kq >= 0) close(el->kq);
@@ -1817,7 +1815,7 @@ fr_event_list_t *fr_event_list_alloc(TALLOC_CTX *ctx, fr_event_status_cb_t statu
        el->kq = -1;    /* So destructor can be used before kqueue() provides us with fd */
        talloc_set_destructor(el, _event_list_free);
 
-       el->times = fr_heap_talloc_create(fr_event_timer_cmp, fr_event_timer_t, heap_id);
+       el->times = fr_heap_talloc_create(el, fr_event_timer_cmp, fr_event_timer_t, heap_id);
        if (!el->times) {
                fr_strerror_printf("Failed allocating event heap");
        error:

diff --git a/src/lib/util/heap.c b/src/lib/util/heap.c
index da0dbe41b4f217e085298d8316e581be88a501bc..0674e1794c25a7c546b2cefb7a60a2b01b56294d 100644 (file)
--- a/src/lib/util/heap.c
+++ b/src/lib/util/heap.c
@@ -58,13 +58,13 @@ struct fr_heap_t {
 
 static void fr_heap_bubble(fr_heap_t *hp, int32_t child);
 
-fr_heap_t *_fr_heap_create(fr_heap_cmp_t cmp, char const *type, size_t offset)
+fr_heap_t *_fr_heap_create(TALLOC_CTX *ctx, fr_heap_cmp_t cmp, char const *type, size_t offset)
 {
        fr_heap_t *fh;
 
        if (!cmp) return NULL;
 
-       fh = talloc_zero(NULL, fr_heap_t);
+       fh = talloc_zero(ctx, fr_heap_t);
        if (!fh) return NULL;
 
        fh->size = 2048;
@@ -332,7 +332,7 @@ int main(int argc, char **argv)
                skip = atoi(argv[1]);
        }
 
-       hp = fr_heap_create(heap_cmp, offsetof(heap_thing, heap_id));
+       hp = fr_heap_create(NULL, heap_cmp, offsetof(heap_thing, heap_id));
        if (!hp) {
                fprintf(stderr, "Failed creating heap!\n");
                fr_exit(1);

diff --git a/src/main/module.c b/src/main/module.c
index 30136d32f5625050356ccc2690062e485ce5b26b..8cf1f69b029fd1c80905e1b2a55a8a6b4999b51b 100644 (file)
--- a/src/main/module.c
+++ b/src/main/module.c
@@ -492,9 +492,8 @@ static void _module_thread_instance_free(void *to_free)
 {
        module_thread_instance_t *ti = talloc_get_type_abort(to_free, module_thread_instance_t);
 
-       if (ti->module->thread_detach) {
-               (void) ti->module->thread_detach(ti->el, ti->data);
-       }
+       DEBUG3("Worker cleaning up %s thread instance data (%p/%p)", ti->module->name, ti, ti->data);
+       if (ti->module->thread_detach) (void) ti->module->thread_detach(ti->el, ti->data);
 
        talloc_free(ti);
 }
@@ -507,6 +506,8 @@ static void _module_thread_inst_tree_free(void *to_free)
 {
        rbtree_t *thread_inst_tree = talloc_get_type_abort(to_free , rbtree_t);
 
+       DEBUG3("Worker cleaning up thread instance tree");
+
        talloc_free(thread_inst_tree);
 }
 
@@ -563,20 +564,18 @@ static int _module_thread_instantiate(void *instance, void *ctx)
                MEM(type_name = talloc_typed_asprintf(NULL, "rlm_%s_thread_t", mi->module->name));
                talloc_set_name(ti->data, "%s", type_name);
                talloc_free(type_name);
-
        }
 
+       DEBUG3("Worker alloced %s thread instance data (%p/%p)", ti->module->name, ti, ti->data);
        if (mi->module->thread_instantiate) {
                ret = mi->module->thread_instantiate(mi->dl_inst->conf, mi->dl_inst->data,
-                                                          thread_inst_ctx->el, ti->data);
+                                                    thread_inst_ctx->el, ti->data);
                if (ret < 0) {
-                       ERROR("Thread instantiation failed for module \"%s\"",
-                             mi->name);
+                       ERROR("Thread instantiation failed for module \"%s\"", mi->name);
                        return -1;
                }
        }
 
-
        rbtree_insert(thread_inst_ctx->tree, ti);
 
        return 0;

diff --git a/src/main/pool.c b/src/main/pool.c
index b6899b83e2d67119d5c51fe9114f5659acd11495..5f5b9ea89c476a50b92bc5b6889813ca74d37d80 100644 (file)
--- a/src/main/pool.c
+++ b/src/main/pool.c
@@ -1000,7 +1000,7 @@ fr_pool_t *fr_pool_init(TALLOC_CTX *ctx,
         *      https://code.facebook.com/posts/1499322996995183/solving-the-mystery-of-link-imbalance-a-metastable-failure-state-at-scale/
         */
        if (!pool->spread) {
-               pool->heap = fr_heap_talloc_create(last_reserved_cmp, fr_pool_connection_t, heap_id);
+               pool->heap = fr_heap_talloc_create(pool, last_reserved_cmp, fr_pool_connection_t, heap_id);
        /*
         *      For some types of connections we need to used a different
         *      algorithm, because load balancing benefits are secondary
@@ -1020,7 +1020,7 @@ fr_pool_t *fr_pool_init(TALLOC_CTX *ctx,
         *      That way we maximise time between connection use.
         */
        } else {
-               pool->heap = fr_heap_talloc_create(last_released_cmp, fr_pool_connection_t, heap_id);
+               pool->heap = fr_heap_talloc_create(pool, last_released_cmp, fr_pool_connection_t, heap_id);
        }
        if (!pool->heap) {
                ERROR("%s: Failed creating connection heap", __FUNCTION__);
@@ -1323,8 +1323,6 @@ void fr_pool_free(fr_pool_t *pool)
                connection_close_internal(pool, NULL, this);
        }
 
-       talloc_free(pool->heap);
-
        fr_pool_trigger_exec(pool, NULL, "stop");
 
        rad_assert(pool->head == NULL);

diff --git a/src/modules/rlm_cache/drivers/rlm_cache_rbtree/rlm_cache_rbtree.c b/src/modules/rlm_cache/drivers/rlm_cache_rbtree/rlm_cache_rbtree.c
index 845dc7e34ce413f3781a03af18e4871c56cb5ffd..56f0dd38232ca1383609cc753a124a54c0df825b 100644 (file)
--- a/src/modules/rlm_cache/drivers/rlm_cache_rbtree/rlm_cache_rbtree.c
+++ b/src/modules/rlm_cache/drivers/rlm_cache_rbtree/rlm_cache_rbtree.c
@@ -86,7 +86,6 @@ static int mod_detach(void *instance)
 {
        rlm_cache_rbtree_t *driver = talloc_get_type_abort(instance, rlm_cache_rbtree_t);
 
-       if (driver->heap) talloc_free(driver->heap);
        if (driver->cache) {
                rbtree_walk(driver->cache, RBTREE_DELETE_ORDER, _cache_entry_free, NULL);
                talloc_free(driver->cache);
@@ -118,7 +117,7 @@ static int mod_instantiate(UNUSED rlm_cache_config_t const *config, void *instan
        /*
         *      The heap of entries to expire.
         */
-       driver->heap = fr_heap_talloc_create(cache_heap_cmp, rlm_cache_rbtree_entry_t, heap_id);
+       driver->heap = fr_heap_talloc_create(driver, cache_heap_cmp, rlm_cache_rbtree_entry_t, heap_id);
        if (!driver->heap) {
                ERROR("Failed to create heap for the cache");
                return -1;

diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c
index 9b8c72619006a1034eb6bb45880e1dc6348cc53b..d7cc035781671c72191635a2edf22ee48a73c902 100644 (file)
--- a/src/modules/rlm_radius/rlm_radius_udp.c
+++ b/src/modules/rlm_radius/rlm_radius_udp.c
@@ -2594,13 +2594,13 @@ static int mod_thread_instantiate(UNUSED CONF_SECTION const *cs, void *instance,
        t->inst = instance;
        t->el = el;
 
-       t->queued = fr_heap_talloc_create(queue_cmp, rlm_radius_udp_request_t, heap_id);
+       t->queued = fr_heap_talloc_create(t, queue_cmp, rlm_radius_udp_request_t, heap_id);
        FR_DLIST_INIT(t->blocked);
        FR_DLIST_INIT(t->full);
        FR_DLIST_INIT(t->zombie);
        FR_DLIST_INIT(t->opening);
 
-       t->active = fr_heap_talloc_create(conn_cmp, rlm_radius_udp_connection_t, heap_id);
+       t->active = fr_heap_talloc_create(t, conn_cmp, rlm_radius_udp_connection_t, heap_id);
 
        conn_alloc(t->inst, t);
 
@@ -2624,8 +2624,6 @@ static int mod_thread_detach(UNUSED fr_event_list_t *el, void *thread)
         *      Free all of the heaps, lists, and sockets.
         */
        talloc_free_children(t);
-       talloc_free(t->queued);
-       talloc_free(t->active);
 
        entry = FR_DLIST_FIRST(t->opening);
        if (entry != NULL) {

diff --git a/src/unlang/interpret.c b/src/unlang/interpret.c
index bfc9fc9b5bdd731de2e06cd697d003e290608a99..6798c558d2353683302c4e73338a380ec467735a 100644 (file)
--- a/src/unlang/interpret.c
+++ b/src/unlang/interpret.c
@@ -773,7 +773,7 @@ rlm_rcode_t unlang_interpret_synchronous(REQUEST *request, CONF_SECTION *cs, rlm
                return RLM_MODULE_FAIL;
        }
 
-       MEM(backlog = fr_heap_talloc_create(_unlang_request_ptr_cmp, REQUEST, runnable_id));
+       MEM(backlog = fr_heap_talloc_create(el, _unlang_request_ptr_cmp, REQUEST, runnable_id));
        old_el = request->el;
        old_backlog = request->backlog;
        caller = request->module;
@@ -806,7 +806,6 @@ rlm_rcode_t unlang_interpret_synchronous(REQUEST *request, CONF_SECTION *cs, rlm
 
        talloc_free(el);
        request->el = old_el;
-       talloc_free(backlog);
        request->backlog = old_backlog;
        request->module = caller;