apr pools & memory leaks (revisited, if briefly)

Some time ago, Ben Collins-Sussman shared with us some of the Google teams'
findings about APR's memory subsystem[1]. In his mail, he says (among other
things) the following:

   Over at Google, we simply hacked APR to *never* hold on to blocks
   for recycling. Essentially, this makes apr_pool_destroy() always
   free() the block, and makes apr_pool_create() always call malloc()
   malloc. Poof, all the memory leak went away instantly.

   What was more troubling is that the use of the MaxMemFree directive --
   which is supposed to limit the total size of the 'free memory'
   recycling list -- didn't seem to work for us. What we need to do is
   go back and debug this more carefully, and see if it's a bug in APR,
   apache, or just in our testing methodology.

I came across this mail thread again recently as part of my own
investigation into Apache's seeming ignorance of the MaxMemFree directive.
So I poked Ben to see if he would/could share the patch to APR that they
wound up with. He responded with the following (and the attached patch,
which I had to reformat to get to a apply cleanly):

   From: Ben Collins-Sussman <sussman_at_google.com>
   Date: Mon, 14 Dec 2009 09:46:45 -0600
   Message-ID: <944683b40912140746t7763f6f8i5af9c549a8b68e90_at_mail.gmail.com>
   Subject: Re: Got Apache insta-free patch?

   Crazy simple patch. Basically, if memory_recycling is disabled,
   then we prevent any freed-up pool from *ever* be pushed on to the
   'free list' of pools to recycle. The result is that all calls to
   apr_pool_destroy() causes the actual unix free() call to happen.
   (And likewise, because the 'free list' of recyclable pools is
   always empty, every call to apr_pool_create() causes malloc() to
   happen.)

   Why this works: apr's recycling logic is ridiculously primitive.
   If there are 50 pools on the 'free list' which are each 2K, and a
   request comes in for a new pool that needs 20k, *none of those
   recyclable pools* get recombined at all. They just sit there
   forever, while apr malloc's a new 20k pool. The result is that for
   really long-running processes, apr just gradually holds on to more
   and more memory over time, ever more fragmented. This isn't a
   problem with normal worker/slave mode, since slave httpd processes
   die and get respawned after N requests. But if you run in
   multithreaded mode (which is the default on Windows, I think), then
   it's just all one big process that runs forever, leaking forever.

   Why we feel this patch is fine for production: the whole apr pool
   system was written in the mid-90's, back when malloc() and free()
   were seriously expensive and made your app noticeably slower. The
   concept of recycling was seen as a way to prevent these expensive
   operations. These days there is *no* speed hit at all that we can
   see. Computers and OSes are just so much faster.

   Feel free to post this to the whole dev_at_subversion list if you
   wish.

I'm "feeling free" to share this with y'all. I'm not vouching for its
accuracy -- I haven't had a chance to test it myself yet. But it's
Christmas time, so have a cup of good cheer and some more free() calls on
Google.

[1] http://svn.haxx.se/dev/archive-2008-10/0070.shtml

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

diff -ru srclib/apr/include/apr_pools.h srclib/apr/include/apr_pools.h
--- srclib/apr/include/apr_pools.h 2008-07-14 11:07:00.000000000 -0400
+++ srclib/apr/include/apr_pools.h 2009-12-14 10:52:16.000000000 -0500
@@ -765,6 +765,14 @@
 
 #endif /* APR_POOL_DEBUG or DOXYGEN */
 
+
+/**
+ * Enable or disable recycling of memory in pools.
+ * @param enable Non-zero to enable recycling, 0 to disable it
+ */
+APR_DECLARE(void) apr_set_memory_recycling(int enable);
+
+
 /** @} */
 
 #ifdef __cplusplus
diff -ru srclib/apr/memory/unix/apr_pools.c srclib/apr/memory/unix/apr_pools.c
--- srclib/apr/memory/unix/apr_pools.c 2008-07-19 07:54:52.000000000 -0400
+++ srclib/apr/memory/unix/apr_pools.c 2009-12-14 10:53:06.000000000 -0500
@@ -40,6 +40,18 @@
 #endif
 
 
+
+/*
+ * Runtime disabling of recycling
+ */
+
+static int memory_recycling = 1;
+
+void apr_set_memory_recycling(int enable)
+{
+ memory_recycling = enable;
+}
+
 /*
  * Magic numbers
  */
@@ -354,8 +366,9 @@
         next = node->next;
         index = node->index;
 
- if (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED
- && index > current_free_index) {
+ if (!memory_recycling ||
+ (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED
+ && index > current_free_index)) {
             node->next = freelist;
             freelist = node;
         }