mpm_common: add pool argument to mpm_register_poll_callback[_timeout] hooks.

This is cleaner API than using pfds->pool implicitely.

MAJOR bump but reusing the existing hooks (with an API/ABI breakage) because
it's trunk material only.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1879449 13f79535-47bb-0310-9956-ffa450edef68

include/ap_mmn.h

@@ -646,14 +646,17 @@
  * 20200420.9 (2.5.1-dev)  Add hooks deliver_report and gather_reports to
  *                         mod_dav.h.
  * 20200420.10 (2.5.1-dev) Add method_precondition hook to mod_dav.h.
- * 20200701.0 (2.5.1-dev)  Axe ap_mpm_unregister_poll_callback and
+ * 20200701.0 (2.5.1-dev)  Axe ap_mpm_unregister_poll_callback() and
  *                         mpm_unregister_poll_callback hook.
+ * 20200702.0 (2.5.1-dev)  Add pool arg to mpm_register_poll_callback and
+ *                         mpm_register_poll_callback_timeout hooks
+ *                         
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20200701
+#define MODULE_MAGIC_NUMBER_MAJOR 20200702
 #endif
 #define MODULE_MAGIC_NUMBER_MINOR 0             /* 0...n */
 

include/ap_mpm.h

@@ -210,6 +210,7 @@ AP_DECLARE(apr_status_t) ap_mpm_register_timed_callback(
 /**
  * Register a callback on the readability or writability on a group of
  * sockets/pipes.
+ * @param p Pool used by the MPM for its internal allocations
  * @param pfds Array of apr_pollfd_t
  * @param cbfn The callback function
  * @param baton userdata for the callback function
@@ -217,14 +218,19 @@ AP_DECLARE(apr_status_t) ap_mpm_register_timed_callback(
  * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
  * @remark When activity is found on any 1 socket/pipe in the list, all are removed
  * from the pollset and only 1 callback is issued.
+ * @remark The passed in pool can be cleared by cbfn and tofn when called back,
+ *         it retains no MPM persistent data and won't be used until the next call
+ *         to ap_mpm_register_poll_callback[_timeout].
  */
 
-AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(apr_array_header_t *pfds,
+AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(
+        apr_pool_t *p, apr_array_header_t *pfds,
         ap_mpm_callback_fn_t *cbfn, void *baton);
 
 /**
  * Register a callback on the readability or writability on a group of sockets/pipes,
  * with a timeout.
+ * @param p Pool used by the MPM for its internal allocations
  * @param pfds Array of apr_pollfd_t
  * @param cbfn The callback function
  * @param tofn The callback function if the timeout expires
@@ -235,11 +241,15 @@ AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(apr_array_header_t *pfds,
  * @remark When activity is found on any 1 socket/pipe in the list, all are removed
  * from the pollset and only 1 callback is issued. 
  * @remark For each call, only one of tofn or cbfn will be called, never both.
+ * @remark The passed in pool can be cleared by cbfn and tofn when called back,
+ *         it retains no MPM persistent data and won't be used until the next call
+ *         to ap_mpm_register_poll_callback[_timeout].
  */
 
 AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback_timeout(
-        apr_array_header_t *pfds, ap_mpm_callback_fn_t *cbfn,
+        apr_pool_t *p, apr_array_header_t *pfds,
-        ap_mpm_callback_fn_t *tofn, void *baton, apr_time_t timeout);
+        ap_mpm_callback_fn_t *cbfn, ap_mpm_callback_fn_t *tofn,
+        void *baton, apr_time_t timeout);
 
 
 typedef enum mpm_child_status {

include/mpm_common.h

@@ -427,18 +427,18 @@ AP_DECLARE_HOOK(apr_status_t, mpm_register_timed_callback,
  * @ingroup hooks
  */
 AP_DECLARE_HOOK(apr_status_t, mpm_register_poll_callback,
-                (apr_array_header_t *pds, ap_mpm_callback_fn_t *cbfn, void *baton))
+                (apr_pool_t *p, apr_array_header_t *pds,
+                 ap_mpm_callback_fn_t *cbfn, void *baton))
 
 /* register the specified callback, with timeout 
  * @ingroup hooks
  *
  */
 AP_DECLARE_HOOK(apr_status_t, mpm_register_poll_callback_timeout,
-        (apr_array_header_t *pds,
+                (apr_pool_t *p, apr_array_header_t *pds,
                 ap_mpm_callback_fn_t *cbfn,
                 ap_mpm_callback_fn_t *tofn,
-                void *baton,
+                void *baton, apr_time_t timeout))
-                apr_time_t timeout))
 
 /** Resume the suspended connection 
  * @ingroup hooks

modules/proxy/mod_proxy_http.c

@@ -254,6 +254,8 @@ typedef struct {
 
     const char *upgrade;
     proxy_tunnel_rec *tunnel;
+
+    apr_pool_t *async_pool;
     apr_array_header_t *pfds;
     apr_interval_time_t idle_timeout;
 
@@ -307,9 +309,9 @@ static void proxy_http_async_cb(void *baton)
     proxy_http_req_t *req = (proxy_http_req_t *)baton;
     int status;
 
-    if (req->pfds) {
+    if (req->async_pool) {
         /* Clear MPM's temporary data */
-        apr_pool_clear(req->pfds->pool);
+        apr_pool_clear(req->async_pool);
     }
 
     switch (req->state) {
@@ -317,14 +319,16 @@ static void proxy_http_async_cb(void *baton)
         /* 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) {
+            if (!req->async_pool) {
                 /* Create the MPM's (req->)pfds off of our tunnel's, and
-                 * overwrite its pool with a subpool since the MPM will use
+                 * the subpool used by the MPM to alloc its own temporary
-                 * that to alloc its own temporary data, which we want to
+                 * data, which we want to clear on the next round (above)
-                 * clear on the next round (above) to avoid leaks.
+                 * to avoid leaks.
                  */
                 req->pfds = apr_array_copy(req->p, req->tunnel->pfds);
-                apr_pool_create(&req->pfds->pool, req->p);
+                APR_ARRAY_IDX(req->pfds, 0, apr_pollfd_t).client_data = NULL;
+                APR_ARRAY_IDX(req->pfds, 1, apr_pollfd_t).client_data = NULL;
+                apr_pool_create(&req->async_pool, req->p);
             }
             else {
                 /* Update only reqevents of the MPM's pfds with our tunnel's,
@@ -352,7 +356,7 @@ static void proxy_http_async_cb(void *baton)
                       "proxy %s: suspended, going async",
                       req->proto);
 
-        ap_mpm_register_poll_callback_timeout(req->pfds,
+        ap_mpm_register_poll_callback_timeout(req->async_pool, req->pfds,
                                               proxy_http_async_cb, 
                                               proxy_http_async_cancel_cb, 
                                               req, req->idle_timeout);

modules/proxy/mod_proxy_wstunnel.c

@@ -30,6 +30,7 @@ typedef struct ws_baton_t {
     proxy_conn_rec *backend;
     proxy_tunnel_rec *tunnel;
     apr_array_header_t *pfds;
+    apr_pool_t *async_pool;
     const char *scheme;
 } ws_baton_t;
 
@@ -83,38 +84,27 @@ static void proxy_wstunnel_cancel_callback(void *b)
 static void proxy_wstunnel_callback(void *b)
 { 
     ws_baton_t *baton = (ws_baton_t*)b;
-    proxyws_dir_conf *dconf = ap_get_module_config(baton->r->per_dir_config,
-                                                   &proxy_wstunnel_module);
 
-    if (baton->pfds) {
+    /* Clear MPM's temporary data */
-        /* Clear MPM's temporary data */
+    AP_DEBUG_ASSERT(baton->async_pool != NULL);
-        apr_pool_clear(baton->pfds->pool);
+    apr_pool_clear(baton->async_pool);
-    }
 
     if (proxy_wstunnel_pump(baton, 1) == SUSPENDED) {
+        proxyws_dir_conf *dconf = ap_get_module_config(baton->r->per_dir_config,
+                                                       &proxy_wstunnel_module);
+
         ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, baton->r,
                       "proxy_wstunnel_callback suspend");
 
-        if (!baton->pfds) {
+        /* Update only reqevents of the MPM's pfds with our tunnel's,
-            /* Create the MPM's (baton->)pfds off of our tunnel's, and
+         * the rest didn't change.
-             * overwrite its pool with a subpool since the MPM will use
+         */
-             * that to alloc its own temporary data, which we want to
+        APR_ARRAY_IDX(baton->pfds, 0, apr_pollfd_t).reqevents =
-             * clear on the next round (above) to avoid leaks.
+            APR_ARRAY_IDX(baton->tunnel->pfds, 0, apr_pollfd_t).reqevents;
-             */
+        APR_ARRAY_IDX(baton->pfds, 1, apr_pollfd_t).reqevents =
-            baton->pfds = apr_array_copy(baton->r->pool, baton->tunnel->pfds);
+            APR_ARRAY_IDX(baton->tunnel->pfds, 1, apr_pollfd_t).reqevents;
-            apr_pool_create(&baton->pfds->pool, baton->r->pool);
-        }
-        else {
-            /* Update only reqevents of the MPM's pfds with our tunnel's,
-             * the rest didn't change.
-             */
-            APR_ARRAY_IDX(baton->pfds, 0, apr_pollfd_t).reqevents =
-                APR_ARRAY_IDX(baton->tunnel->pfds, 0, apr_pollfd_t).reqevents;
-            APR_ARRAY_IDX(baton->pfds, 1, apr_pollfd_t).reqevents =
-                APR_ARRAY_IDX(baton->tunnel->pfds, 1, apr_pollfd_t).reqevents;
-        }
 
-        ap_mpm_register_poll_callback_timeout(baton->pfds,
+        ap_mpm_register_poll_callback_timeout(baton->async_pool, baton->pfds,
                                               proxy_wstunnel_callback, 
                                               proxy_wstunnel_cancel_callback, 
                                               baton, dconf->idle_timeout);
@@ -286,14 +276,17 @@ static int proxy_wstunnel_request(apr_pool_t *p, request_rec *r,
         status = proxy_wstunnel_pump(baton, 1);
         if (status == SUSPENDED) {
             /* Create the MPM's (baton->)pfds off of our tunnel's, and
-             * overwrite its pool with a subpool since the MPM will use
+             * the subpool used by the MPM to alloc its own temporary
-             * that to alloc its own temporary data, which we want to
+             * data, which we want to clear on the next round (above)
-             * clear on the next round (above) to avoid leaks.
+             * to avoid leaks.
              */
             baton->pfds = apr_array_copy(baton->r->pool, baton->tunnel->pfds);
-            apr_pool_create(&baton->pfds->pool, baton->r->pool);
+            APR_ARRAY_IDX(baton->pfds, 0, apr_pollfd_t).client_data = NULL;
+            APR_ARRAY_IDX(baton->pfds, 1, apr_pollfd_t).client_data = NULL;
+            apr_pool_create(&baton->async_pool, baton->r->pool);
 
-            rv = ap_mpm_register_poll_callback_timeout(baton->pfds,
+            rv = ap_mpm_register_poll_callback_timeout(
+                         baton->async_pool, baton->pfds,
                          proxy_wstunnel_callback, 
                          proxy_wstunnel_cancel_callback, 
                          baton, 

server/mpm/event/event.c

@@ -1604,13 +1604,13 @@ static apr_status_t event_cleanup_poll_callback(void *data)
     return final_rc;
 }
 
-static apr_status_t event_register_poll_callback_ex(apr_array_header_t *pfds,
+static apr_status_t event_register_poll_callback_ex(apr_pool_t *p,
-                                                  ap_mpm_callback_fn_t *cbfn,
+                                                    apr_array_header_t *pfds,
-                                                  ap_mpm_callback_fn_t *tofn,
+                                                    ap_mpm_callback_fn_t *cbfn,
-                                                  void *baton,
+                                                    ap_mpm_callback_fn_t *tofn,
-                                                  apr_time_t timeout)
+                                                    void *baton,
+                                                    apr_time_t timeout)
 {
-    apr_pool_t *p = pfds->pool;
     socket_callback_baton_t *scb = apr_pcalloc(p, sizeof(*scb));
     listener_poll_type *pt = apr_palloc(p, sizeof(*pt));
     apr_status_t rc, final_rc = APR_SUCCESS;
@@ -1655,11 +1655,13 @@ static apr_status_t event_register_poll_callback_ex(apr_array_header_t *pfds,
     return final_rc;
 }
 
-static apr_status_t event_register_poll_callback(apr_array_header_t *pfds,
+static apr_status_t event_register_poll_callback(apr_pool_t *p,
+                                                 apr_array_header_t *pfds,
                                                  ap_mpm_callback_fn_t *cbfn,
                                                  void *baton)
 {
-    return event_register_poll_callback_ex(pfds,
+    return event_register_poll_callback_ex(p,
+                                           pfds,
                                            cbfn,
                                            NULL, /* no timeout function */
                                            baton,
@@ -4073,10 +4075,10 @@ static void event_hooks(apr_pool_t * p)
     ap_hook_mpm_query(event_query, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_mpm_register_timed_callback(event_register_timed_callback, NULL, NULL,
                                         APR_HOOK_MIDDLE);
-    ap_hook_mpm_register_poll_callback(event_register_poll_callback, NULL, NULL,
+    ap_hook_mpm_register_poll_callback(event_register_poll_callback,
-                                        APR_HOOK_MIDDLE);
+                                       NULL, NULL, APR_HOOK_MIDDLE);
-    ap_hook_mpm_register_poll_callback_timeout(event_register_poll_callback_ex, NULL, NULL,
+    ap_hook_mpm_register_poll_callback_timeout(event_register_poll_callback_ex,
-                                        APR_HOOK_MIDDLE);
+                                               NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_pre_read_request(event_pre_read_request, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_post_read_request(event_post_read_request, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_mpm_get_name(event_get_name, NULL, NULL, APR_HOOK_MIDDLE);

server/mpm_common.c

@@ -110,11 +110,15 @@ AP_IMPLEMENT_HOOK_RUN_FIRST(apr_status_t, mpm_resume_suspended,
                             (conn_rec *c),
                             (c), APR_ENOTIMPL)
 AP_IMPLEMENT_HOOK_RUN_FIRST(apr_status_t, mpm_register_poll_callback,
-                            (apr_array_header_t *pds, ap_mpm_callback_fn_t *cbfn, void *baton),
+                            (apr_pool_t *p, apr_array_header_t *pds,
-                            (pds, cbfn, baton), APR_ENOTIMPL)
+                             ap_mpm_callback_fn_t *cbfn, void *baton),
+                            (p, pds, cbfn, baton), APR_ENOTIMPL)
 AP_IMPLEMENT_HOOK_RUN_FIRST(apr_status_t, mpm_register_poll_callback_timeout,
-                            (apr_array_header_t *pds, ap_mpm_callback_fn_t *cbfn, ap_mpm_callback_fn_t *tofn, void *baton, apr_time_t timeout),
+                            (apr_pool_t *p, apr_array_header_t *pds,
-                            (pds, cbfn, tofn, baton, timeout), APR_ENOTIMPL)
+                             ap_mpm_callback_fn_t *cbfn,
+                             ap_mpm_callback_fn_t *tofn,
+                             void *baton, apr_time_t timeout),
+                            (p, pds, cbfn, tofn, baton, timeout), APR_ENOTIMPL)
 AP_IMPLEMENT_HOOK_RUN_FIRST(int, output_pending,
                             (conn_rec *c), (c), DECLINED)
 AP_IMPLEMENT_HOOK_RUN_FIRST(int, input_pending,
@@ -567,18 +571,19 @@ AP_DECLARE(apr_status_t) ap_mpm_register_timed_callback(apr_time_t t,
     return ap_run_mpm_register_timed_callback(t, cbfn, baton);
 }
 
-AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(apr_array_header_t *pfds,
+AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(
+        apr_pool_t *p, apr_array_header_t *pfds,
         ap_mpm_callback_fn_t *cbfn, void *baton)
 {
-    return ap_run_mpm_register_poll_callback(pfds, cbfn, baton);
+    return ap_run_mpm_register_poll_callback(p, pfds, cbfn, baton);
 }
 
 AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback_timeout(
-        apr_array_header_t *pfds, ap_mpm_callback_fn_t *cbfn,
+        apr_pool_t *p, apr_array_header_t *pfds, ap_mpm_callback_fn_t *cbfn,
         ap_mpm_callback_fn_t *tofn, void *baton, apr_time_t timeout)
 {
-    return ap_run_mpm_register_poll_callback_timeout(pfds, cbfn, tofn, baton,
+    return ap_run_mpm_register_poll_callback_timeout(p, pfds, cbfn, tofn,
-            timeout);
+                                                     baton, timeout);
 }
 
 AP_DECLARE(const char *)ap_show_mpm(void)