You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2018/02/01 15:01:41 UTC
svn commit: r1822879 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Author: ylavic
Date: Thu Feb 1 15:01:40 2018
New Revision: 1822879
URL: http://svn.apache.org/viewvc?rev=1822879&view=rev
Log:
mod_proxy: follow up to r1822849 and r1822878.
Does r1822878's "static" APR_RETRIEVE_OPTIONAL_FN work if, say, mod_proxy is
builtin but mod_http2 isn't?
Not worth taking the risk here since it's not a fast path...
Note: if this is an issue, I'm afraid it applies elsewhere too.
Modified:
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1822879&r1=1822878&r2=1822879&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 1 15:01:40 2018
@@ -1816,17 +1816,10 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sha
PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, server_rec *s, apr_pool_t *p)
{
- static int have_get_h2_num_workers = 0;
- static APR_OPTIONAL_FN_TYPE(http2_get_num_workers)
- *get_h2_num_workers = NULL;
+ APR_OPTIONAL_FN_TYPE(http2_get_num_workers) *get_h2_num_workers;
apr_status_t rv = APR_SUCCESS;
int max_threads, minw, maxw;
- if (get_h2_num_workers == NULL) {
- get_h2_num_workers = APR_RETRIEVE_OPTIONAL_FN(http2_get_num_workers);
- have_get_h2_num_workers = (get_h2_num_workers != NULL);
- }
-
if (worker->s->status & PROXY_WORKER_INITIALIZED) {
/* The worker is already initialized */
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00924)
@@ -1854,7 +1847,8 @@ PROXY_DECLARE(apr_status_t) ap_proxy_ini
* since it has it's own pool of processing threads.
*/
ap_mpm_query(AP_MPMQ_MAX_THREADS, &max_threads);
- if (have_get_h2_num_workers) {
+ get_h2_num_workers = APR_RETRIEVE_OPTIONAL_FN(http2_get_num_workers);
+ if (get_h2_num_workers) {
get_h2_num_workers(s, &minw, &maxw);
if (max_threads < maxw) {
max_threads = maxw;
Re: svn commit: r1822879 -
/httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Feb 05, 2018 at 09:06:09AM -0600, William A Rowe Jr wrote:
> You can't retrieve in the register fn hook, without creating load order
> dependencies.
Yup, that's why there is ap_hook_optional_fn_retrieve which is the right
place to do it.
Re: svn commit: r1822879 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
You can't retrieve in the register fn hook, without creating load order
dependencies.
On Feb 2, 2018 02:44, "Joe Orton" <jo...@redhat.com> wrote:
> On Thu, Feb 01, 2018 at 03:01:41PM -0000, ylavic@apache.org wrote:
> > Author: ylavic
> > Date: Thu Feb 1 15:01:40 2018
> > New Revision: 1822879
> >
> > URL: http://svn.apache.org/viewvc?rev=1822879&view=rev
> > Log:
> > mod_proxy: follow up to r1822849 and r1822878.
> >
> > Does r1822878's "static" APR_RETRIEVE_OPTIONAL_FN work if, say,
> mod_proxy is
> > builtin but mod_http2 isn't?
>
> I'd guess not!
>
> > Not worth taking the risk here since it's not a fast path...
> >
> > Note: if this is an issue, I'm afraid it applies elsewhere too.
>
> The only places I found with optional fn declared static within a
> function were both pointlessly static and safe, fixed in r1822931. It's
> funny since we have a specific hook for retrieving optional functions at
> the right time, but it's almost never used. Most modules do it in
> post_config which is safe.
>
> Regards, Joe
>
Re: svn commit: r1822879 -
/httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Feb 01, 2018 at 03:01:41PM -0000, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Feb 1 15:01:40 2018
> New Revision: 1822879
>
> URL: http://svn.apache.org/viewvc?rev=1822879&view=rev
> Log:
> mod_proxy: follow up to r1822849 and r1822878.
>
> Does r1822878's "static" APR_RETRIEVE_OPTIONAL_FN work if, say, mod_proxy is
> builtin but mod_http2 isn't?
I'd guess not!
> Not worth taking the risk here since it's not a fast path...
>
> Note: if this is an issue, I'm afraid it applies elsewhere too.
The only places I found with optional fn declared static within a
function were both pointlessly static and safe, fixed in r1822931. It's
funny since we have a specific hook for retrieving optional functions at
the right time, but it's almost never used. Most modules do it in
post_config which is safe.
Regards, Joe