Re: Time for 1.3.3

[Bojan Smojver]

On Fri, 2008-08-08 at 00:42 +0200, Peter Poeml wrote:

> I am a heavy user of both mod_dbd and apr_memcache, and I'm
> investigating an apr_memcache related memleak right now that I started
> getting with 1.3.0, so I will give this a good testing in the next few
> hours.

Latest patch for memcache leak is attached.

-- 
Bojan

Index: memcache/apr_memcache.c
===================================================================
--- memcache/apr_memcache.c	(revision 683718)
+++ memcache/apr_memcache.c	(working copy)
@@ -295,7 +295,28 @@
     return rv;
 }
 
+static apr_status_t conn_clean(void *data)
+{
+    apr_memcache_conn_t *conn = data;
+    struct iovec vec[2];
+    apr_size_t written;
 
+    /* send a quit message to the memcached server to be nice about it. */
+    vec[0].iov_base = MC_QUIT;
+    vec[0].iov_len = MC_QUIT_LEN;
+
+    vec[1].iov_base = MC_EOL;
+    vec[1].iov_len = MC_EOL_LEN;
+    
+    /* Return values not checked, since we just want to make it go away. */
+    apr_socket_sendv(conn->sock, vec, 2, &written);
+    apr_socket_close(conn->sock);
+
+    conn->p = NULL; /* so that destructor does not destroy the pool again */
+
+    return APR_SUCCESS;
+}
+
 static apr_status_t
 mc_conn_construct(void **conn_, void *params, apr_pool_t *pool)
 {
@@ -310,7 +331,11 @@
         return rv;
     }
 
+#if APR_HAS_THREADS
+    conn = malloc(sizeof( apr_memcache_conn_t )); /* non-pool space! */
+#else
     conn = apr_palloc(np, sizeof( apr_memcache_conn_t ));
+#endif
 
     conn->p = np;
 
@@ -334,8 +359,10 @@
     rv = conn_connect(conn);
     if (rv != APR_SUCCESS) {
         apr_pool_destroy(np);
+        free(conn);
     }
     else {
+        apr_pool_cleanup_register(np, conn, conn_clean, apr_pool_cleanup_null);
         *conn_ = conn;
     }
     
@@ -347,20 +374,13 @@
 mc_conn_destruct(void *conn_, void *params, apr_pool_t *pool)
 {
     apr_memcache_conn_t *conn = (apr_memcache_conn_t*)conn_;
-    struct iovec vec[2];
-    apr_size_t written;
     
-    /* send a quit message to the memcached server to be nice about it. */
-    vec[0].iov_base = MC_QUIT;
-    vec[0].iov_len = MC_QUIT_LEN;
+    if (conn->p) {
+        apr_pool_destroy(conn->p);
+    }
 
-    vec[1].iov_base = MC_EOL;
-    vec[1].iov_len = MC_EOL_LEN;
+    free(conn); /* free non-pool space */
     
-    /* Return values not checked, since we just want to make it go away. */
-    apr_socket_sendv(conn->sock, vec, 2, &written);
-    apr_socket_close(conn->sock);
-    
     return APR_SUCCESS;
 }
 #endif