osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: svn commit: r1841225 - /httpd/httpd/trunk/modules/dav/main/props.c


Gentle ping :-). Did you find some time to have a look?

Regards

Rüdiger

On 09/25/2018 06:37 PM, Greg Stein wrote:
> We learned a lot about pool handling while writing Subversion, after I wrote that mod_dav code. There are definite some
> improvements to be made. I'm not surprised that a propfind can go nuts like that ... I'll review the change and take a
> look generally.
> 
> h/t to DanielR for the pointer to this thread.
> 
> 
> On Tue, Sep 18, 2018 at 8:04 AM Ruediger Pluem <rpluem@xxxxxxxxxx <mailto:rpluem@xxxxxxxxxx>> wrote:
> 
>     Pools are very tricky in mod_dav. Hence additional eyeballs are very much welcome here.
>     As I only did testing with mod_dav_fs I would be keen to know if things still work with Subversion.
>     So if someone from the Subversion guys is listening here: Having this tested with Subversion would be very welcome :-).
> 
>     Regards
> 
>     Rüdiger
> 
>     On 09/18/2018 02:58 PM, rpluem@xxxxxxxxxx <mailto:rpluem@xxxxxxxxxx> wrote:
>     > Author: rpluem
>     > Date: Tue Sep 18 12:58:57 2018
>     > New Revision: 1841225
>     >
>     > URL: http://svn.apache.org/viewvc?rev=1841225&view=rev
>     > Log:
>     > * Doing a PROPFIND on a large collection e.g. 50.000 elements can easily
>     >   consume 1 GB of memory as the subrequests and propdb pools are not
>     >   destroyed and cleared after each element was handled.
>     >   Do this now. There is one case in dav_get_props where elem->priv
>     >   lives longer then the propdb pool. In this case allocate from r->pool.
>     >   Furthermore also recycle propdb's which allows to clear the propdb's
>     >   pools instead of destroying them and creating them again.
>     >
>     > Modified:
>     >     httpd/httpd/trunk/modules/dav/main/props.c
>     >
>     > Modified: httpd/httpd/trunk/modules/dav/main/props.c
>     > URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/props.c?rev=1841225&r1=1841224&r2=1841225&view=diff
>     > ==============================================================================
>     > --- httpd/httpd/trunk/modules/dav/main/props.c (original)
>     > +++ httpd/httpd/trunk/modules/dav/main/props.c Tue Sep 18 12:58:57 2018
>     > @@ -524,7 +524,21 @@ DAV_DECLARE(dav_error *)dav_open_propdb(
>     >                                          apr_array_header_t * ns_xlate,
>     >                                          dav_propdb **p_propdb)
>     >  {
>     > -    dav_propdb *propdb = apr_pcalloc(r->pool, sizeof(*propdb));
>     > +    dav_propdb *propdb = NULL;
>     > +    /*
>     > +     * Check if we have tucked away a previous propdb and reuse it.
>     > +     * Otherwise create a new one and tuck it away
>     > +     */
>     > +    apr_pool_userdata_get((void **)&propdb, "propdb", r->pool);
>     > +    if (!propdb) {
>     > +        propdb = apr_pcalloc(r->pool, sizeof(*propdb));
>     > +        apr_pool_userdata_setn(propdb, "propdb", NULL, r->pool);
>     > +        apr_pool_create(&propdb->p, r->pool);
>     > +    }
>     > +    else {
>     > +        /* Play safe and clear the pool of the reused probdb */
>     > +        apr_pool_clear(propdb->p);
>     > +    }
>     > 
>     >      *p_propdb = NULL;
>     > 
>     > @@ -537,7 +551,6 @@ DAV_DECLARE(dav_error *)dav_open_propdb(
>     >  #endif
>     > 
>     >      propdb->r = r;
>     > -    apr_pool_create(&propdb->p, r->pool);
>     >      propdb->resource = resource;
>     >      propdb->ns_xlate = ns_xlate;
>     > 
>     > @@ -562,10 +575,12 @@ DAV_DECLARE(void) dav_close_propdb(dav_p
>     >          (*propdb->db_hooks->close)(propdb->db);
>     >      }
>     > 
>     > -    /* Currently, mod_dav's pool usage doesn't allow clearing this pool. */
>     > -#if 0
>     > -    apr_pool_destroy(propdb->p);
>     > -#endif
>     > +    if (propdb->subreq) {
>     > +        ap_destroy_sub_req(propdb->subreq);
>     > +        propdb->subreq = NULL;
>     > +    }
>     > +
>     > +    apr_pool_clear(propdb->p);
>     >  }
>     > 
>     >  DAV_DECLARE(dav_get_props_result) dav_get_allprops(dav_propdb *propdb,
>     > @@ -815,7 +830,8 @@ DAV_DECLARE(dav_get_props_result) dav_ge
>     >          */
>     > 
>     >          if (elem->priv == NULL) {
>     > -            elem->priv = apr_pcalloc(propdb->p, sizeof(*priv));
>     > +            /* elem->priv outlives propdb->p. Hence use the request pool */
>     > +            elem->priv = apr_pcalloc(propdb->r->pool, sizeof(*priv));
>     >          }
>     >          priv = elem->priv;
>     > 
>     >
>     >
>     >
>