You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2020/07/02 12:22:07 UTC
svn commit: r1879438 - /httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
Author: ylavic
Date: Thu Jul 2 12:22:07 2020
New Revision: 1879438
URL: http://svn.apache.org/viewvc?rev=1879438&view=rev
Log:
mod_proxy_wstunnel: follow up to r1879418: handle first async lifetime too.
Create the dedicated pfds and subpool in proxy_wstunnel_request() too, for
the first call to ap_mpm_register_poll_callback_timeout().
While at it, add comments about why we need the dedicated pfds/subpool.
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=1879438&r1=1879437&r2=1879438&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c Thu Jul 2 12:22:07 2020
@@ -87,6 +87,7 @@ static void proxy_wstunnel_callback(void
&proxy_wstunnel_module);
if (baton->pfds) {
+ /* Clear MPM's temporary data */
apr_pool_clear(baton->pfds->pool);
}
@@ -94,16 +95,24 @@ static void proxy_wstunnel_callback(void
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;
- }
- else {
+ if (!baton->pfds) {
+ /* Create the MPM's (baton->)pfds off of our tunnel's, and
+ * overwrite its pool with a subpool since the MPM will use
+ * that to alloc its own temporary data, which we want to
+ * clear on the next round (above) to avoid leaks.
+ */
baton->pfds = apr_array_copy(baton->r->pool, baton->tunnel->pfds);
apr_pool_create(&baton->pfds->pool, baton->r->pool);
}
+ else {
+ /* Update only reqevents of the MPM's pfds with our tunnel's,
+ * the rest didn't change.
+ */
+ APR_ARRAY_IDX(baton->pfds, 0, apr_pollfd_t).reqevents =
+ APR_ARRAY_IDX(baton->tunnel->pfds, 0, apr_pollfd_t).reqevents;
+ APR_ARRAY_IDX(baton->pfds, 1, apr_pollfd_t).reqevents =
+ APR_ARRAY_IDX(baton->tunnel->pfds, 1, apr_pollfd_t).reqevents;
+ }
ap_mpm_register_poll_callback_timeout(baton->pfds,
proxy_wstunnel_callback,
@@ -276,7 +285,15 @@ static int proxy_wstunnel_request(apr_po
tunnel->timeout = dconf->async_delay;
status = proxy_wstunnel_pump(baton, 1);
if (status == SUSPENDED) {
- rv = ap_mpm_register_poll_callback_timeout(tunnel->pfds,
+ /* Create the MPM's (baton->)pfds off of our tunnel's, and
+ * overwrite its pool with a subpool since the MPM will use
+ * that to alloc its own temporary data, which we want to
+ * clear on the next round (above) to avoid leaks.
+ */
+ baton->pfds = apr_array_copy(baton->r->pool, baton->tunnel->pfds);
+ apr_pool_create(&baton->pfds->pool, baton->r->pool);
+
+ rv = ap_mpm_register_poll_callback_timeout(baton->pfds,
proxy_wstunnel_callback,
proxy_wstunnel_cancel_callback,
baton,
@@ -284,7 +301,8 @@ static int proxy_wstunnel_request(apr_po
if (rv == APR_SUCCESS) {
return SUSPENDED;
}
- else if (APR_STATUS_IS_ENOTIMPL(rv)) {
+
+ if (APR_STATUS_IS_ENOTIMPL(rv)) {
ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, APLOGNO(02544) "No async support");
tunnel->timeout = dconf->idle_timeout;
status = proxy_wstunnel_pump(baton, 0); /* force no async */
Re: svn commit: r1879438 - /httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Jul 2, 2020 at 2:32 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 7/2/20 2:22 PM, ylavic@apache.org wrote:
> > Author: ylavic
> > Date: Thu Jul 2 12:22:07 2020
> > New Revision: 1879438
> >
> > URL: http://svn.apache.org/viewvc?rev=1879438&view=rev
> > Log:
> > mod_proxy_wstunnel: follow up to r1879418: handle first async lifetime too.
> >
> > Create the dedicated pfds and subpool in proxy_wstunnel_request() too, for
> > the first call to ap_mpm_register_poll_callback_timeout().
> >
> > While at it, add comments about why we need the dedicated pfds/subpool.
> >
> >
> > 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=1879438&r1=1879437&r2=1879438&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c Thu Jul 2 12:22:07 2020
>
> > @@ -94,16 +95,24 @@ static void proxy_wstunnel_callback(void
> > 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;
> > - }
> > - else {
> > + if (!baton->pfds) {
>
> Can this still happen with the changes below?
No, fixed in r1879449.
Thanks;
Yann.
Re: svn commit: r1879438 -
/httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 7/2/20 2:22 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Jul 2 12:22:07 2020
> New Revision: 1879438
>
> URL: http://svn.apache.org/viewvc?rev=1879438&view=rev
> Log:
> mod_proxy_wstunnel: follow up to r1879418: handle first async lifetime too.
>
> Create the dedicated pfds and subpool in proxy_wstunnel_request() too, for
> the first call to ap_mpm_register_poll_callback_timeout().
>
> While at it, add comments about why we need the dedicated pfds/subpool.
>
>
> 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=1879438&r1=1879437&r2=1879438&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c Thu Jul 2 12:22:07 2020
> @@ -94,16 +95,24 @@ static void proxy_wstunnel_callback(void
> 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;
> - }
> - else {
> + if (!baton->pfds) {
Can this still happen with the changes below?
> + /* Create the MPM's (baton->)pfds off of our tunnel's, and
> + * overwrite its pool with a subpool since the MPM will use
> + * that to alloc its own temporary data, which we want to
> + * clear on the next round (above) to avoid leaks.
> + */
> baton->pfds = apr_array_copy(baton->r->pool, baton->tunnel->pfds);
> apr_pool_create(&baton->pfds->pool, baton->r->pool);
> }
> + else {
> + /* Update only reqevents of the MPM's pfds with our tunnel's,
> + * the rest didn't change.
> + */
> + APR_ARRAY_IDX(baton->pfds, 0, apr_pollfd_t).reqevents =
> + APR_ARRAY_IDX(baton->tunnel->pfds, 0, apr_pollfd_t).reqevents;
> + APR_ARRAY_IDX(baton->pfds, 1, apr_pollfd_t).reqevents =
> + APR_ARRAY_IDX(baton->tunnel->pfds, 1, apr_pollfd_t).reqevents;
> + }
>
> ap_mpm_register_poll_callback_timeout(baton->pfds,
> proxy_wstunnel_callback,
Regards
Rüdiger