You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by bo...@apache.org on 2008/08/08 01:47:12 UTC
svn commit: r683756 - /apr/apr-util/branches/1.3.x/memcache/apr_memcache.c
Author: bojan
Date: Thu Aug 7 16:47:12 2008
New Revision: 683756
URL: http://svn.apache.org/viewvc?rev=683756&view=rev
Log:
Fix memcache memory leak without use of pre_cleanup in reslist.
Modified:
apr/apr-util/branches/1.3.x/memcache/apr_memcache.c
Modified: apr/apr-util/branches/1.3.x/memcache/apr_memcache.c
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.3.x/memcache/apr_memcache.c?rev=683756&r1=683755&r2=683756&view=diff
==============================================================================
--- apr/apr-util/branches/1.3.x/memcache/apr_memcache.c (original)
+++ apr/apr-util/branches/1.3.x/memcache/apr_memcache.c Thu Aug 7 16:47:12 2008
@@ -295,6 +295,27 @@
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,19 +374,12 @@
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;
-
- /* 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);
+ free(conn); /* free non-pool space */
return APR_SUCCESS;
}
Re: svn commit: r683756 - /apr/apr-util/branches/1.3.x/memcache/apr_memcache.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, Aug 7, 2008 at 7:47 PM, <bo...@apache.org> wrote:
> Author: bojan
> Date: Thu Aug 7 16:47:12 2008
> New Revision: 683756
>
> URL: http://svn.apache.org/viewvc?rev=683756&view=rev
> Log:
> Fix memcache memory leak without use of pre_cleanup in reslist.
>
> Modified:
> apr/apr-util/branches/1.3.x/memcache/apr_memcache.c
>
> Modified: apr/apr-util/branches/1.3.x/memcache/apr_memcache.c
> URL:
> http://svn.apache.org/viewvc/apr/apr-util/branches/1.3.x/memcache/apr_memcache.c?rev=683756&r1=683755&r2=683756&view=diff
>
> ==============================================================================
> --- apr/apr-util/branches/1.3.x/memcache/apr_memcache.c (original)
> +++ apr/apr-util/branches/1.3.x/memcache/apr_memcache.c Thu Aug 7 16:47:12
> 2008
> @@ -295,6 +295,27 @@
> 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);
>
This needs to be inside #if APR_HAS_THREADS.
@@ -347,19 +374,12 @@
> mc_conn_destruct(void *conn_, void *params, apr_pool_t *pool)
> {
...
> - /* 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);
> + free(conn); /* free non-pool space */
already inside APR_HAS_THREADS
--
Born in Roswell... married an alien...