From aec9f4a5dd80da7d58cca5ebfd1da4af39e4d7c2 Mon Sep 17 00:00:00 2001 From: Ruediger Pluem Date: Tue, 2 Jul 2019 09:08:30 +0000 Subject: [PATCH] Merge r1842010, r1841225, r1862039, r1862040, r1862042 from trunk: * dav_stream_response processes data that has been allocated from the propdb pool. Hence close the propdb *after* dav_stream_response which clears thei probdb pool. * 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. Simplify handling of short-lived pool for dav_propdb in mod_dav. No functional change. * modules/dav/main/props.c (dav_popen_propdb): Rename from dav_open_propdb, take a pool argument. (dav_open_propdb): Reimplement in terms of above, using r->pool. (dav_propfind_walker): Switch to using dav_open_propdb with scratchpool. * modules/dav/main/props.c (dav_do_prop_subreq): Allocate escaped URI out of propdb pool, fixing small per-resource leak during a PROPFIND walk. Submitted by: jorton, rpluem * modules/dav/main/mod_dav.c (dav_send_multistatus): Tag the pool. Reviewed by: rpluem, jorton, jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1862410 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 +++ STATUS | 10 ---------- include/ap_mmn.h | 3 ++- modules/dav/main/mod_dav.c | 6 ++++-- modules/dav/main/mod_dav.h | 10 ++++++++++ modules/dav/main/props.c | 29 +++++++++++++++++++++-------- 6 files changed, 40 insertions(+), 21 deletions(-) diff --git a/CHANGES b/CHANGES index 24adfedaefa..7ef828cf1bb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.40 + *) mod_dav: Reduce the amount of memory needed when doing PROPFIND's on large + collections by improving the memory management. [Joe Orton, Ruediger Pluem] + *) mod_proxy_http2: adding support for handling trailers in both directions. PR 63502. [Stefan Eissing] diff --git a/STATUS b/STATUS index 4e5a60cf6f4..fbbd01c8051 100644 --- a/STATUS +++ b/STATUS @@ -127,16 +127,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - *) mod_dav: Reduce the amount of memory needed when doing PROPFIND's on large - collections by improving the memory management. - [Joe Orton, Ruediger Pluem] - trunk patch: http://svn.apache.org/r1842010 - http://svn.apache.org/r1841225 - http://svn.apache.org/r1862039 - http://svn.apache.org/r1862040 - http://svn.apache.org/r1862042 - 2.4.x patch: svn merge -c 1842010,1841225,1862039,1862040,1862042 ^/httpd/httpd/trunk . - +1: rpluem, jorton, jim PATCHES PROPOSED TO BACKPORT FROM TRUNK: diff --git a/include/ap_mmn.h b/include/ap_mmn.h index e47d0a4d9c0..a29db02dcd8 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -527,6 +527,7 @@ * core_server_conf. * 20120211.85 (2.4.40-dev) add ap_set_conn_count(). * 20120211.86 (2.4.35-dev) Add forward_100_continue{,_set} to proxy_dir_conf + * 20120211.87 (2.4.40-dev) Add dav_popen_propdb */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ @@ -534,7 +535,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20120211 #endif -#define MODULE_MAGIC_NUMBER_MINOR 86 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 87 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c index 7e316cb13ba..6a36bfe67d0 100644 --- a/modules/dav/main/mod_dav.c +++ b/modules/dav/main/mod_dav.c @@ -557,6 +557,7 @@ DAV_DECLARE(void) dav_send_multistatus(request_rec *r, int status, dav_begin_multistatus(bb, r, status, namespaces); apr_pool_create(&subpool, r->pool); + apr_pool_tag(subpool, "mod_dav-multistatus"); for (; first != NULL; first = first->next) { apr_pool_clear(subpool); @@ -1980,8 +1981,9 @@ static dav_error * dav_propfind_walker(dav_walk_resource *wres, int calltype) ** Note: we cast to lose the "const". The propdb won't try to change ** the resource, however, since we are opening readonly. */ - err = dav_open_propdb(ctx->r, ctx->w.lockdb, wres->resource, 1, - ctx->doc ? ctx->doc->namespaces : NULL, &propdb); + err = dav_popen_propdb(ctx->scratchpool, + ctx->r, ctx->w.lockdb, wres->resource, 1, + ctx->doc ? ctx->doc->namespaces : NULL, &propdb); if (err != NULL) { /* ### do something with err! */ diff --git a/modules/dav/main/mod_dav.h b/modules/dav/main/mod_dav.h index 80ad1176b4d..1641804670f 100644 --- a/modules/dav/main/mod_dav.h +++ b/modules/dav/main/mod_dav.h @@ -1590,6 +1590,16 @@ DAV_DECLARE(dav_error *) dav_open_propdb( apr_array_header_t *ns_xlate, dav_propdb **propdb); +DAV_DECLARE(dav_error *) dav_popen_propdb( + apr_pool_t *p, + request_rec *r, + dav_lockdb *lockdb, + const dav_resource *resource, + int ro, + apr_array_header_t *ns_xlate, + dav_propdb **propdb); + + DAV_DECLARE(void) dav_close_propdb(dav_propdb *db); DAV_DECLARE(dav_get_props_result) dav_get_props( diff --git a/modules/dav/main/props.c b/modules/dav/main/props.c index 47cc509bd47..47adfc4f08a 100644 --- a/modules/dav/main/props.c +++ b/modules/dav/main/props.c @@ -323,7 +323,7 @@ static void dav_do_prop_subreq(dav_propdb *propdb) { /* need to escape the uri that's in the resource struct because during * the property walker it's not encoded. */ - const char *e_uri = ap_escape_uri(propdb->resource->pool, + const char *e_uri = ap_escape_uri(propdb->p, propdb->resource->uri); /* perform a "GET" on the resource's URI (note that the resource @@ -524,7 +524,20 @@ DAV_DECLARE(dav_error *)dav_open_propdb(request_rec *r, dav_lockdb *lockdb, apr_array_header_t * ns_xlate, dav_propdb **p_propdb) { - dav_propdb *propdb = apr_pcalloc(r->pool, sizeof(*propdb)); + return dav_popen_propdb(r->pool, r, lockdb, resource, ro, ns_xlate, p_propdb); +} + +DAV_DECLARE(dav_error *)dav_popen_propdb(apr_pool_t *p, + request_rec *r, dav_lockdb *lockdb, + const dav_resource *resource, + int ro, + apr_array_header_t * ns_xlate, + dav_propdb **p_propdb) +{ + dav_propdb *propdb = NULL; + + propdb = apr_pcalloc(p, sizeof(*propdb)); + propdb->p = p; *p_propdb = NULL; @@ -537,7 +550,6 @@ DAV_DECLARE(dav_error *)dav_open_propdb(request_rec *r, dav_lockdb *lockdb, #endif propdb->r = r; - apr_pool_create(&propdb->p, r->pool); propdb->resource = resource; propdb->ns_xlate = ns_xlate; @@ -562,10 +574,10 @@ DAV_DECLARE(void) dav_close_propdb(dav_propdb *propdb) (*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; + } } DAV_DECLARE(dav_get_props_result) dav_get_allprops(dav_propdb *propdb, @@ -739,7 +751,8 @@ DAV_DECLARE(dav_get_props_result) dav_get_props(dav_propdb *propdb, */ 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; -- 2.47.3