On Thu, Jul 2, 2020 at 10:17 AM Ruediger Pluem <[email protected]> wrote:
>
> On 7/2/20 2:14 AM, [email protected] wrote:
> >
> > +/* Invoked by the event loop when data is ready on either end.
> > + * We don't need the invoke_mtx, since we never put multiple callback
> > events
> > + * in the queue.
> > + */
> > +static void proxy_http_async_cb(void *baton)
> > +{
> > + proxy_http_req_t *req = (proxy_http_req_t *)baton;
> > + int status;
> > +
> > + if (req->pfds) {
> > + apr_pool_clear(req->pfds->pool);
> > + }
> > +
> > + switch (req->state) {
> > + case PROXY_HTTP_TUNNELING:
> > + /* Pump both ends until they'd block and then start over again */
> > + status = ap_proxy_tunnel_run(req->tunnel);
> > + if (status == HTTP_GATEWAY_TIME_OUT) {
> > + if (req->pfds) {
> > + apr_pollfd_t *async_pfds = (void *)req->pfds->elts;
> > + apr_pollfd_t *tunnel_pfds = (void
> > *)req->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.
Here and in mod_proxy_wstunnel, the goal is that we have a dedicated
pfds array for ap_mpm_register_poll_callback_timeout() which modifies
the array in-place and uses pfds->pool for its own allocations (see
event_register_poll_callback_ex() in MPM event).
async_pfds and tunnel_pfds are local but point to req->pfds and
req->tunnel->pfd, this is just to avoid ugly one-line casting.
I could use the APR_ARRAY_IDX() macro though..
>
> > + }
> > + else {
> > + req->pfds = apr_array_copy(req->p, req->tunnel->pfds);
> > + apr_pool_create(&req->pfds->pool, req->p);
>
> Why first using baton->r->pool to create the copy and then setting the pool
> of the array to the new pool?
Since event_register_poll_callback_ex() uses pfds->pool for mpm_event
"internal" allocations, creating and clearing subpool req->pfds->pool
avoids leaks when proxy_http_async_cb() (and thus
ap_mpm_register_poll_callback_timeout() below) is called multiple
times, each time connections are idle and need rescheduling through
the MPM.
>
> > + }
> > + status = SUSPENDED;
> > + }
> > + break;
> > +
> > + default:
> > + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, req->r,
> > + "proxy %s: unexpected async state (%i)",
> > + req->proto, (int)req->state);
> > + status = HTTP_INTERNAL_SERVER_ERROR;
> > + break;
> > + }
> > +
> > + if (status == SUSPENDED) {
> > + ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, req->r,
> > + "proxy %s: suspended, going async",
> > + req->proto);
> > +
> > + ap_mpm_register_poll_callback_timeout(req->pfds,
> > + proxy_http_async_cb,
> > + proxy_http_async_cancel_cb,
> > + req, req->idle_timeout);
> > + }
> > + else if (status != OK) {
> > + proxy_http_async_cancel_cb(req);
> > + }
> > + else {
> > + proxy_http_async_finish(req);
> > + }
> > +}
Regards;
Yann.
