On 6/18/24 4:20 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Tue Jun 18 14:20:06 2024
> New Revision: 1918412
>
> URL: http://svn.apache.org/viewvc?rev=1918412&view=rev
> Log:
> mod_proxy: Keep connection alive on addressTTL expiry if the DNS didn't
> change.
>
> * modules/proxy/proxy_util.c(address_cleanup):
> Rename to conn_cleanup() since it also closes the socket, and run
> socket_cleanup() first to avoid dangling conn->sock->remote_addr.
>
> * modules/proxy/proxy_util.c(ap_proxy_determine_address):
> Compare the new address with the old one and keep the socket alive
> if it did not change.
>
> Follow up to r1918410.
>
>
> Modified:
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1918412&r1=1918411&r2=1918412&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Jun 18 14:20:06 2024
> static apr_status_t conn_pool_cleanup(void *theworker)
> @@ -3004,13 +3002,25 @@ PROXY_DECLARE(apr_status_t) ap_proxy_det
>
> PROXY_THREAD_UNLOCK(worker);
>
> - /* Kill any socket using the old address */
> - if (conn->sock) {
> - if (r ? APLOGrdebug(r) : APLOGdebug(s)) {
> - /* XXX: this requires the old conn->addr[ess] to still
> - * be alive since it's not copied by apr_socket_connect()
> - * in ap_proxy_connect_backend().
> - */
> + /* Release the old conn address */
> + if (conn->address) {
> + /* conn->address->addr cannot be released while it's used by
> + * conn->sock->remote_addr, thus if the old address is still
> + * the one used at apr_socket_connect() time AND the socket
> + * shouldn't be closed because the newly resolved address is
> + * the same. In this case let's (re)attach the old address to
> + * the socket lifetime (scpool), in any other case just
> release
> + * it now.
> + */
> + int addr_alive = 0,
> + conn_alive = (conn->sock && conn->addr &&
> + proxy_addrs_equal(conn->addr,
> address->addr));
> + if (conn_alive) {
> + apr_sockaddr_t *remote_addr = NULL;
> + apr_socket_addr_get(&remote_addr, APR_REMOTE,
> conn->sock);
> + addr_alive = (conn->addr == remote_addr);
How can this ever be true? The remote_addr is always allocated from
conn->scpool. How can
conn->addr ever be allocated from conn->scpool? I think it always gets
allocated from
either a subpool of worker->cp->dns_pool, from conn->pool or from
conn->uds_pool.
> + }
> + else if (conn->sock && (r ? APLOGrdebug(r) : APLOGdebug(s)))
> {
> apr_sockaddr_t *local_addr = NULL;
> apr_sockaddr_t *remote_addr = NULL;
> apr_socket_addr_get(&local_addr, APR_LOCAL, conn->sock);
Regards
Rüdiger
