Re: svn commit: r1879418 - /httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1879418 - /httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c

Ruediger Pluem


On 7/2/20 2:07 AM, [hidden email] wrote:

> Author: ylavic
> Date: Thu Jul  2 00:07:04 2020
> New Revision: 1879418
>
> URL: http://svn.apache.org/viewvc?rev=1879418&view=rev
> Log:
> mod_proxy_wstunnel: avoid leaks on tunnel->pfds->pool.
>
> Since event_register_poll_callback_ex() allocates its data on pfds->pool,
> we need a subpool to be cleared at each proxy_wstunnel_callback() call.
>
>
> Modified:
>     httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c?rev=1879418&r1=1879417&r2=1879418&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c Thu Jul  2 00:07:04 2020

> @@ -84,15 +85,30 @@ static void proxy_wstunnel_callback(void
>      ws_baton_t *baton = (ws_baton_t*)b;
>      proxyws_dir_conf *dconf = ap_get_module_config(baton->r->per_dir_config,
>                                                     &proxy_wstunnel_module);
> -    int status = proxy_wstunnel_pump(baton, 1);
> -    if (status == SUSPENDED) {
> -        ap_mpm_register_poll_callback_timeout(baton->tunnel->pfds,
> -            proxy_wstunnel_callback,
> -            proxy_wstunnel_cancel_callback,
> -            baton,
> -            dconf->idle_timeout);
> +
> +    if (baton->pfds) {
> +        apr_pool_clear(baton->pfds->pool);
> +    }
> +
> +    if (proxy_wstunnel_pump(baton, 1) == SUSPENDED) {
>          ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, baton->r,
>                        "proxy_wstunnel_callback suspend");
> +
> +        if (baton->pfds) {
> +            apr_pollfd_t *async_pfds = (void *)baton->pfds->elts;
> +            apr_pollfd_t *tunnel_pfds = (void *)baton->tunnel->pfds->elts;
> +            async_pfds[0].reqevents = tunnel_pfds[0].reqevents;
> +            async_pfds[1].reqevents = tunnel_pfds[1].reqevents;

What is the purpose of this?
async_pfds and tunnel_pfds  are local to this block and cannot be used outside this block.


> +        }
> +        else {
> +            baton->pfds = apr_array_copy(baton->r->pool, baton->tunnel->pfds);
> +            apr_pool_create(&baton->pfds->pool, baton->r->pool);

Why first using baton->r->pool to create the copy and then setting the pool of the array to the new pool?

> +        }
> +
> +        ap_mpm_register_poll_callback_timeout(baton->pfds,
> +                                              proxy_wstunnel_callback,
> +                                              proxy_wstunnel_cancel_callback,
> +                                              baton, dconf->idle_timeout);
>      }
>      else {
>          proxy_wstunnel_finish(baton);
>
>
>

Regards

RĂ¼diger