diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 9bd6b01b84..68905ad85b 100644 --- a/include/ap_mmn.h +++ b/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 */ diff --git a/include/ap_mpm.h b/include/ap_mpm.h index f446587836..a5e2ce1062 100644 --- a/include/ap_mpm.h +++ b/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, - ap_mpm_callback_fn_t *tofn, void *baton, apr_time_t timeout); + 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); typedef enum mpm_child_status { diff --git a/include/mpm_common.h b/include/mpm_common.h index 673c470064..a0966a58ba 100644 --- a/include/mpm_common.h +++ b/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, - apr_time_t timeout)) + void *baton, apr_time_t timeout)) /** Resume the suspended connection * @ingroup hooks diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 6a778bf229..3572650a0f 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/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 - * that to alloc its own temporary data, which we want to - * clear on the next round (above) to avoid leaks. + * the subpool used by the MPM to alloc its own temporary + * data, which we want to clear on the next round (above) + * 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); diff --git a/modules/proxy/mod_proxy_wstunnel.c b/modules/proxy/mod_proxy_wstunnel.c index e4b32e238e..c98b99ea6b 100644 --- a/modules/proxy/mod_proxy_wstunnel.c +++ b/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 */ - apr_pool_clear(baton->pfds->pool); - } + /* Clear MPM's temporary data */ + AP_DEBUG_ASSERT(baton->async_pool != NULL); + 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) { - /* Create the MPM's (baton->)pfds off of our tunnel's, and - * overwrite its pool with a subpool since the MPM will use - * that to alloc its own temporary data, which we want to - * clear on the next round (above) to avoid leaks. - */ - baton->pfds = apr_array_copy(baton->r->pool, baton->tunnel->pfds); - 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; - } + /* 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 - * that to alloc its own temporary data, which we want to - * clear on the next round (above) to avoid leaks. + * the subpool used by the MPM to alloc its own temporary + * data, which we want to clear on the next round (above) + * 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, diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 4ef72b328f..3de751f36d 100644 --- a/server/mpm/event/event.c +++ b/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, - ap_mpm_callback_fn_t *cbfn, - ap_mpm_callback_fn_t *tofn, - void *baton, - apr_time_t timeout) +static apr_status_t event_register_poll_callback_ex(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) { - 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, - APR_HOOK_MIDDLE); - ap_hook_mpm_register_poll_callback_timeout(event_register_poll_callback_ex, NULL, NULL, - APR_HOOK_MIDDLE); + ap_hook_mpm_register_poll_callback(event_register_poll_callback, + NULL, NULL, APR_HOOK_MIDDLE); + ap_hook_mpm_register_poll_callback_timeout(event_register_poll_callback_ex, + 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); diff --git a/server/mpm_common.c b/server/mpm_common.c index f63f84538d..09a7e06e4a 100644 --- a/server/mpm_common.c +++ b/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), - (pds, cbfn, baton), APR_ENOTIMPL) + (apr_pool_t *p, apr_array_header_t *pds, + 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), - (pds, cbfn, tofn, baton, timeout), APR_ENOTIMPL) + (apr_pool_t *p, apr_array_header_t *pds, + 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, - timeout); + return ap_run_mpm_register_poll_callback_timeout(p, pfds, cbfn, tofn, + baton, timeout); } AP_DECLARE(const char *)ap_show_mpm(void)