This fixes a slow memory leak in mod_proxy FYI. The sockaddr passed to apr_socket_connect() is allocated out of worker->cp->pool. When a new backend connection is created, core_create_conn extracts the address from that socket to the conn_rec and it gets duped in that pool again.
On Mon, Aug 09, 2010 at 12:51:29PM -0000, [email protected] wrote: > Author: jorton > Date: Mon Aug 9 12:51:29 2010 > New Revision: 983618 > > URL: http://svn.apache.org/viewvc?rev=983618&view=rev > Log: > * network_io/unix/sockets.c (apr_socket_connect): Copy the remote > address by value rather than by reference. This ensures that the > sockaddr object returned by apr_socket_addr_get is allocated from > the same pool as the socket object itself, as apr_socket_accept > does; avoiding any potential lifetime mismatches. > > * test/testsock.c (test_get_addr): Enhance test case to cover this. > > Modified: > apr/apr/trunk/network_io/unix/sockets.c > apr/apr/trunk/test/testsock.c > > Modified: apr/apr/trunk/network_io/unix/sockets.c > URL: > http://svn.apache.org/viewvc/apr/apr/trunk/network_io/unix/sockets.c?rev=983618&r1=983617&r2=983618&view=diff > ============================================================================== > --- apr/apr/trunk/network_io/unix/sockets.c (original) > +++ apr/apr/trunk/network_io/unix/sockets.c Mon Aug 9 12:51:29 2010 > @@ -387,10 +387,13 @@ apr_status_t apr_socket_connect(apr_sock > /* A real remote address was passed in. If the unspecified > * address was used, the actual remote addr will have to be > * determined using getpeername() if required. */ > - /* ### this should probably be a structure copy + fixup as per > - * _accept()'s handling of local_addr */ > - sock->remote_addr = sa; > sock->remote_addr_unknown = 0; > + > + /* Copy the address structure details in. */ > + sock->remote_addr->sa = sa->sa; > + sock->remote_addr->salen = sa->salen; > + /* Adjust ipaddr_ptr et al. */ > + apr_sockaddr_vars_set(sock->remote_addr, sa->family, sa->port); > } > > if (sock->local_addr->port == 0) { > > Modified: apr/apr/trunk/test/testsock.c > URL: > http://svn.apache.org/viewvc/apr/apr/trunk/test/testsock.c?rev=983618&r1=983617&r2=983618&view=diff > ============================================================================== > --- apr/apr/trunk/test/testsock.c (original) > +++ apr/apr/trunk/test/testsock.c Mon Aug 9 12:51:29 2010 > @@ -334,8 +334,11 @@ static void test_get_addr(abts_case *tc, > apr_status_t rv; > apr_socket_t *ld, *sd, *cd; > apr_sockaddr_t *sa, *ca; > + apr_pool_t *subp; > char *a, *b; > > + APR_ASSERT_SUCCESS(tc, "create subpool", apr_pool_create(&subp, p)); > + > ld = setup_socket(tc); > > APR_ASSERT_SUCCESS(tc, > @@ -343,7 +346,7 @@ static void test_get_addr(abts_case *tc, > apr_socket_addr_get(&sa, APR_LOCAL, ld)); > > rv = apr_socket_create(&cd, sa->family, SOCK_STREAM, > - APR_PROTO_TCP, p); > + APR_PROTO_TCP, subp); > APR_ASSERT_SUCCESS(tc, "create client socket", rv); > > APR_ASSERT_SUCCESS(tc, "enable non-block mode", > @@ -369,7 +372,7 @@ static void test_get_addr(abts_case *tc, > } > > APR_ASSERT_SUCCESS(tc, "accept connection", > - apr_socket_accept(&sd, ld, p)); > + apr_socket_accept(&sd, ld, subp)); > > { > /* wait for writability */ > @@ -389,18 +392,38 @@ static void test_get_addr(abts_case *tc, > > APR_ASSERT_SUCCESS(tc, "get local address of server socket", > apr_socket_addr_get(&sa, APR_LOCAL, sd)); > - > APR_ASSERT_SUCCESS(tc, "get remote address of client socket", > apr_socket_addr_get(&ca, APR_REMOTE, cd)); > - > - a = apr_psprintf(p, "%pI", sa); > - b = apr_psprintf(p, "%pI", ca); > > + /* Test that the pool of the returned sockaddr objects exactly > + * match the socket. */ > + ABTS_PTR_EQUAL(tc, subp, sa->pool); > + ABTS_PTR_EQUAL(tc, subp, ca->pool); > + > + /* Check equivalence. */ > + a = apr_psprintf(p, "%pI fam=%d", sa, sa->family); > + b = apr_psprintf(p, "%pI fam=%d", ca, ca->family); > ABTS_STR_EQUAL(tc, a, b); > + > + /* Check pool of returned sockaddr, as above. */ > + APR_ASSERT_SUCCESS(tc, "get local address of client socket", > + apr_socket_addr_get(&sa, APR_LOCAL, cd)); > + APR_ASSERT_SUCCESS(tc, "get remote address of server socket", > + apr_socket_addr_get(&ca, APR_REMOTE, sd)); > + > + /* Check equivalence. */ > + a = apr_psprintf(p, "%pI fam=%d", sa, sa->family); > + b = apr_psprintf(p, "%pI fam=%d", ca, ca->family); > + ABTS_STR_EQUAL(tc, a, b); > + > + ABTS_PTR_EQUAL(tc, subp, sa->pool); > + ABTS_PTR_EQUAL(tc, subp, ca->pool); > > apr_socket_close(cd); > apr_socket_close(sd); > apr_socket_close(ld); > + > + apr_pool_destroy(subp); > } > > static void test_wait(abts_case *tc, void *data) >
