On Thu, Jul 2, 2020 at 2:04 PM Ruediger Pluem <[email protected]> wrote: > > On 7/2/20 12:42 PM, Eric Covener wrote: > > On Thu, Jul 2, 2020 at 6:25 AM Ruediger Pluem <[email protected]> wrote: > >> > >> On 7/2/20 11:17 AM, Yann Ylavic wrote: > >>> On Thu, Jul 2, 2020 at 10:17 AM Ruediger Pluem <[email protected]> wrote: > >>>> > >>>> On 7/2/20 2:14 AM, [email protected] wrote: > >>>>> > > >>> 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. > >> > >> I understand why you do this, but I think it is a dangerous approach. If > >> the array would do resize operations it would allocate > >> from a different pool then. I think it is a design flaw of > >> event_register_poll_callback_ex to allocate stuff from pfds->pool. > >> It should have a separate pool argument and should use this pool for these > >> allocations. If callers want to have the same > >> lifetime of these objects as the array they could simply supply > >> pfds->pool. I would be even fine if > >> event_register_poll_callback_ex would accept NULL for this parameter and > >> use pfds->pool in this case. > > > > Since these API's never made it out of trunk, I think we can break/fix > > them without too much worry, especially if it simplifies callers. > > +1.
OK, I was going to ask for it, adding (yet) another poll_callback hook looked overkill to me :) I just committed the clarification/comments about the dedicated pfds and subpool used in both proxy_http and proxy_wstunnel (resp. r1879437 and r1879438), going to change to an explicit pool arg now. Thanks for the review! Regards; Yann. > > Regards > > Rüdiger >
