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

diff --git a/CHANGES b/CHANGES
index 24adfedaefa43d696f67398fefb4a69d9f52b771..7ef828cf1bb0fb9936f91baafb60bcc7f699ed80 100644 (file)
--- 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 4e5a60cf6f4008e02b606cbf270c15a8459b2fee..fbbd01c8051ffb2c740c78c7a80eff1293b23dae 100644 (file)
--- 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 e47d0a4d9c03707e6c266ad9ea5befafb918ef0e..a29db02dcd83d6237fc74e20f573eb1dfd360720 100644 (file)
--- 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 7e316cb13ba0e21eebebaeb6f765b320ac239eb3..6a36bfe67d034155026151e24788198516e1820f 100644 (file)
--- 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 80ad1176b4d3a243f57e26cdbd00fcf0e81ddaa2..1641804670f720b817a95f84253a0d1845350558 100644 (file)
--- 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 47cc509bd470c0963138a28da56ca25ac8c656cc..47adfc4f08a95b83114adbe9f94a281d9d56e047 100644 (file)
--- 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;