httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: svn commit: r983618 - in /apr/apr/trunk: network_io/unix/sockets.c test/testsock.c
Date Mon, 09 Aug 2010 13:13:37 GMT
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, jorton@apache.org 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)
> 

Mime
View raw message