Reuse cleanup structures to prevent memory consumption from repeated

register/kill of a cleanup against a single pool:

* memory/unix/apr_pools.c (struct apr_pool_t): Add freelist for
cleanup structures.
(apr_pool_cleanup_kill): Move used cleanup structures onto the
freelist.
(apr_pool_cleanup_register): Reuse cleanup structure if available.
(apr_pool_clear, pool_clear_debug, apr_pool_create): Clear the freelist.

* test/testpools.c (checker_cleanup, success_cleanup, test_cleanups):
Add tests for cleanups.

PR: 23567 (the easy half)


git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@65206 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 62 insertions, 1 deletions

diff --git a/memory/unix/apr_pools.c b/memory/unix/apr_pools.c
index 536854679..2ed524eb3 100644
--- a/memory/unix/apr_pools.c
+++ b/memory/unix/apr_pools.c
@@ -435,6 +435,7 @@ struct apr_pool_t {
     apr_pool_t           *sibling;
     apr_pool_t          **ref;
     cleanup_t            *cleanups;
+    cleanup_t            *free_cleanups;
     apr_allocator_t      *allocator;
     struct process_chain *subprocesses;
     apr_abortfunc_t       abort_fn;
@@ -674,6 +675,7 @@ APR_DECLARE(void) apr_pool_clear(apr_pool_t *pool)
     /* Run cleanups */
     run_cleanups(&pool->cleanups);
     pool->cleanups = NULL;
+    pool->free_cleanups = NULL;
 
     /* Free subprocesses */
     free_proc_chain(pool->subprocesses);
@@ -801,6 +803,7 @@ APR_DECLARE(apr_status_t) apr_pool_create_ex(apr_pool_t **newpool,
     pool->abort_fn = abort_fn;
     pool->child = NULL;
     pool->cleanups = NULL;
+    pool->free_cleanups = NULL;
     pool->subprocesses = NULL;
     pool->user_data = NULL;
     pool->tag = NULL;
@@ -1340,6 +1343,7 @@ static void pool_clear_debug(apr_pool_t *pool, const char *file_line)
 
     /* Run cleanups */
     run_cleanups(&pool->cleanups);
+    pool->free_cleanups = NULL;
     pool->cleanups = NULL;
 
     /* If new child pools showed up, this is a reason to raise a flag */
@@ -1886,7 +1890,13 @@ APR_DECLARE(void) apr_pool_cleanup_register(apr_pool_t *p, const void *data,
 #endif /* APR_POOL_DEBUG */
 
     if (p != NULL) {
-        c = (cleanup_t *)apr_palloc(p, sizeof(cleanup_t));
+        if (p->free_cleanups) {
+            /* reuse a cleanup structure */
+            c = p->free_cleanups;
+            p->free_cleanups = c->next;
+        } else {
+            c = apr_palloc(p, sizeof(cleanup_t));
+        }
         c->data = data;
         c->plain_cleanup_fn = plain_cleanup_fn;
         c->child_cleanup_fn = child_cleanup_fn;
@@ -1912,6 +1922,9 @@ APR_DECLARE(void) apr_pool_cleanup_kill(apr_pool_t *p, const void *data,
     while (c) {
         if (c->data == data && c->plain_cleanup_fn == cleanup_fn) {
             *lastp = c->next;
+            /* move to freelist */
+            c->next = p->free_cleanups;
+            p->free_cleanups = c;
             break;
         }
 
diff --git a/test/testpools.c b/test/testpools.c
index 18f2aa820..5ef6a7ef3 100644
--- a/test/testpools.c
+++ b/test/testpools.c
@@ -91,6 +91,53 @@ static void test_notancestor(abts_case *tc, void *data)
     ABTS_INT_EQUAL(tc, 0, apr_pool_is_ancestor(pchild, pmain));
 }
 
+static apr_status_t success_cleanup(void *data)
+{
+    return APR_SUCCESS;
+}
+
+static char *checker_data = "Hello, world.";
+
+static apr_status_t checker_cleanup(void *data)
+{
+    return data == checker_data ? APR_SUCCESS : APR_EGENERAL;
+}
+
+static void test_cleanups(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    int n;
+
+    /* do this several times to test the cleanup freelist handling. */
+    for (n = 0; n < 5; n++) {
+        apr_pool_cleanup_register(pchild, NULL, success_cleanup,
+                                  success_cleanup);
+        apr_pool_cleanup_register(pchild, checker_data, checker_cleanup,
+                                  success_cleanup);
+        apr_pool_cleanup_register(pchild, NULL, checker_cleanup, 
+                                  success_cleanup);
+
+        rv = apr_pool_cleanup_run(p, NULL, success_cleanup);
+        ABTS_ASSERT(tc, "nullop cleanup run OK", rv == APR_SUCCESS);
+        rv = apr_pool_cleanup_run(p, checker_data, checker_cleanup);
+        ABTS_ASSERT(tc, "cleanup passed correct data", rv == APR_SUCCESS);
+        rv = apr_pool_cleanup_run(p, NULL, checker_cleanup);
+        ABTS_ASSERT(tc, "cleanup passed correct data", rv == APR_EGENERAL);
+
+        if (n == 2) {
+            /* clear the pool to check that works */
+            apr_pool_clear(pchild);
+        }
+
+        if (n % 2 == 0) {
+            /* throw another random cleanup into the mix */
+            apr_pool_cleanup_register(pchild, NULL,
+                                      apr_pool_cleanup_null,
+                                      apr_pool_cleanup_null);
+        }
+    }
+}
+
 abts_suite *testpool(abts_suite *suite)
 {
     suite = ADD_SUITE(suite)
@@ -101,6 +148,7 @@ abts_suite *testpool(abts_suite *suite)
     abts_run_test(suite, test_notancestor, NULL);
     abts_run_test(suite, alloc_bytes, NULL);
     abts_run_test(suite, calloc_bytes, NULL);
+    abts_run_test(suite, test_cleanups, NULL);
 
     return suite;
 }