You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2023/06/29 15:08:03 UTC
Re: svn commit: r1909401 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On Tue, Apr 25, 2023 at 1:57 PM <rp...@apache.org> wrote:
>
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Apr 25 11:57:22 2023
> @@ -2790,7 +2790,11 @@ ap_proxy_determine_connection(apr_pool_t
> * The single DNS lookup is used once per worker.
> * If dynamic change is needed then set the addr to NULL
> * inside dynamic config to force the lookup.
> + *
> + * Clear the dns_pool before to avoid a memory leak in case
> + * we did the lookup again.
> */
> + apr_pool_clear(worker->cp->dns_pool);
> err = apr_sockaddr_info_get(&addr,
> conn->hostname, APR_UNSPEC,
> conn->port, 0,
I'm not sure we can clear the dns_pool, some conn->addr allocated on
it may be still in use by other threads.
While reviewing your backport proposal, I noticed that
worker->cp->addr was used concurrently in several places in mod_proxy
with no particular care, so I started to code a follow up, but it
needs that apr_pool_clear() too somewhere to avoid the leak and
figured it may not be safe (like the above).
I attach the patch here to show what I mean by insecure
worker->cp->addr usage if we start to modify it concurrently, though
more work is needed it seems..
If I am correct we need to refcount the worker->cp->addr (or rather a
worker->address struct). I had a patch which does that to handle a
proxy address_ttl parameter (above which address is renewed), I can
try to revive and mix it with the attached one, in a PR. WDYT?
Regards;
Yann.
Re: svn commit: r1909401 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 7/5/23 2:31 PM, Yann Ylavic wrote:
> On Fri, Jun 30, 2023 at 4:08 PM Yann Ylavic <yl...@gmail.com> wrote:
>>
>> On Fri, Jun 30, 2023 at 2:35 PM Ruediger Pluem <rp...@apache.org> wrote:
>>>
>>> On 6/29/23 5:08 PM, Yann Ylavic wrote:
>>>> On Tue, Apr 25, 2023 at 1:57 PM <rp...@apache.org> wrote:
>>>>>
>>>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>>>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Apr 25 11:57:22 2023
>>>>> @@ -2790,7 +2790,11 @@ ap_proxy_determine_connection(apr_pool_t
>>>>> * The single DNS lookup is used once per worker.
>>>>> * If dynamic change is needed then set the addr to NULL
>>>>> * inside dynamic config to force the lookup.
>>>>> + *
>>>>> + * Clear the dns_pool before to avoid a memory leak in case
>>>>> + * we did the lookup again.
>>>>> */
>>>>> + apr_pool_clear(worker->cp->dns_pool);
>>>>> err = apr_sockaddr_info_get(&addr,
>>>>> conn->hostname, APR_UNSPEC,
>>>>> conn->port, 0,
>>>>
>>>> I'm not sure we can clear the dns_pool, some conn->addr allocated on
>>>> it may be still in use by other threads.
>>>> While reviewing your backport proposal, I noticed that
>>>> worker->cp->addr was used concurrently in several places in mod_proxy
>>>> with no particular care, so I started to code a follow up, but it
>>>> needs that apr_pool_clear() too somewhere to avoid the leak and
>>>> figured it may not be safe (like the above).
>>>> I attach the patch here to show what I mean by insecure
>>>> worker->cp->addr usage if we start to modify it concurrently, though
>>>> more work is needed it seems..
>>>>
>>>> If I am correct we need to refcount the worker->cp->addr (or rather a
>>>> worker->address struct). I had a patch which does that to handle a
>>>> proxy address_ttl parameter (above which address is renewed), I can
>>>> try to revive and mix it with the attached one, in a PR. WDYT?
>>>
>>> Thanks for the good catch. With regards to mod_proxy_ajp I think we should
>>> use conn->addr instead of worker->cp->addr. We cannot be sure that worker->cp->addr is really set.
>>> I guess it did not cause any problems so far as in the AJP case we always reuse connections by default
>>> and hence worker->cp->addr is set.
>>>
>>> I think using a refcount could be burdensome as we need to ensure correct increase and decrease of the refcount and
>>> we might have 3rd party modules that do not play well. Hence I propose a copy approach (but thinking of it, it might
>>> fail in the same or other ways with 3rd party modules using worker->cp->addr directly in a similar way like mod_proxy_ajp does today)
>>
>> Let me look at what I can come up with and compare with what you
>> propose below (which does not look trivial either from a quick look).
>> That is to say, I need a bit more time to have an opinion here :)
>
> So I went with https://github.com/apache/httpd/pull/367 (sorry, not
> much detailed commits yet), and specifically the first commit for the
> reuse / ttl / force-expiring of the worker address(es).
>
> The new function is (named after ap_proxy_determine_connection):
>
> /*
> * Resolve an address, reusing the one of the worker if any.
> * @param proxy_function calling proxy scheme (http, ajp, ...)
> * @param conn proxy connection the address is used for
> * @param hostname host to resolve (should be the worker's if reusable)
> * @param hostport port to resolve (should be the worker's if reusable)
> * @param r current request (if any)
> * @param s current server (or NULL if r != NULL and ap_proxyerror()
> * should be called on error)
> * @return APR_SUCCESS or an error
> */
> PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char
> *proxy_function,
> proxy_conn_rec *conn,
> const char *hostname,
> apr_port_t hostport,
> request_rec *r,
> server_rec *s);
>
> and it takes a proxy_conn_rec only, which simplifies things and which
> we can provide everywhere after all (see the changes in mod_proxy_ftp
> and mod_proxy_hcheck).
>
> A new struct proxy_address (refcounted, TTL'ed) is defined to store an
> address (when needed only, i.e. worker->is_address_reusable). It's
> allocated as a subpool of worker->cp->dns_pool and looks like this:
> struct proxy_address {
> apr_sockaddr_t *addr; /* Remote address info */
> const char *hostname; /* Remote host name */
> apr_port_t hostport; /* Remote host port */
> apr_uint32_t refcount; /* Number of conns and/or worker using it */
> apr_uint32_t expiry; /* Expiry timestamp (seconds to
> proxy_start_time) */
> };
> Each "reusable" proxy_worker and proxy_conn_rec owns a refcount to its
> current address such that the last owner destroys the old one when
> replacing it with the new one, which all happens in
> ap_proxy_determine_address().
>
> There are 2 ways an address can expire, either its TTL is over
> (configured in the worker's addressTTL= parameter), or by forcing
> (conn->)address->expiry = 0 (e.g. when ap_proxy_connect_backend()
> tries to reconnect with a new DNS lookup, per your changes in
> r1909451). There is a proxy_address_set_expired() helper for the
> latter case to synchronize and avoid lookup storms.
>
> The address expiry time is shared between all the children processes
> in worker->s->address_expiry, even though there is a "copy" in each
> address->expiry field to allow for forcing the DNS lookup and
> synchronizing per child.
> Because I didn't want to introduce the first use of the apr_atomic_*64
> API in httpd (not sure which first APR version introduced it and only
> the latest versions are reliable anyway), the expiry is 32bit
> timestamp in seconds relative to some new proxy_start_time epoch
> (initialized at httpd startup). The timestamps are taken from
> apr_time_now() which is not a monotonic clock so it's not ideal (don't
> change your system's time while httpd is running!), I think we need an
> ap_monotonic_now() but that's another story (which I drafted in PR
> #294 for mpm_event FWIW).
>
> Since conn->addr and conn->hostname (i.e. conn->address->*) always
> point to some valid memory (can be NULL before
> ap_proxy_determine_address() still), they can be used safely during
> connection processing regardless of worker->address changing
> underneath, the eventual change will be taken into account in the
> proxy_conn_rec the next time it's reused (closing the "old" socket if
> any).
>
> Regarding worker->s->addr, it's now legacy and set by
> ap_proxy_determine_address() only if it's NULL (that is at startup or
> if the user explicitly NULLed it). The first address' refcount is set
> to 2 such that worker->s->addr is never deallocated, users can
> continue to use it although it will not necessarily be the latest/real
> address in use. That way we do not break users, and they can switch to
> using conn->addr instead if needed.
> Note that *worker->address is never safe to use outside a
> PROXY_THREAD_[UN]LOCK() critical section, but changing *conn->address
> is and does the same if it's the current address, which is useful for
> the safety/correctness (hopefully) of ap_proxy_determine_address()
> BTW.
>
> There are other changes in the first commit for the lifetime of the
> conn->uds_path and conn->forward, but the other commits are not really
> related (I happened to make these changes while walking the code..).
>
> WDYT?
Thank you very much for the amount of work you did on it. I asked questions / gave comments in PR.
Regards
Rüdiger
Re: svn commit: r1909401 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Jun 30, 2023 at 4:08 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Fri, Jun 30, 2023 at 2:35 PM Ruediger Pluem <rp...@apache.org> wrote:
> >
> > On 6/29/23 5:08 PM, Yann Ylavic wrote:
> > > On Tue, Apr 25, 2023 at 1:57 PM <rp...@apache.org> wrote:
> > >>
> > >> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > >> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Apr 25 11:57:22 2023
> > >> @@ -2790,7 +2790,11 @@ ap_proxy_determine_connection(apr_pool_t
> > >> * The single DNS lookup is used once per worker.
> > >> * If dynamic change is needed then set the addr to NULL
> > >> * inside dynamic config to force the lookup.
> > >> + *
> > >> + * Clear the dns_pool before to avoid a memory leak in case
> > >> + * we did the lookup again.
> > >> */
> > >> + apr_pool_clear(worker->cp->dns_pool);
> > >> err = apr_sockaddr_info_get(&addr,
> > >> conn->hostname, APR_UNSPEC,
> > >> conn->port, 0,
> > >
> > > I'm not sure we can clear the dns_pool, some conn->addr allocated on
> > > it may be still in use by other threads.
> > > While reviewing your backport proposal, I noticed that
> > > worker->cp->addr was used concurrently in several places in mod_proxy
> > > with no particular care, so I started to code a follow up, but it
> > > needs that apr_pool_clear() too somewhere to avoid the leak and
> > > figured it may not be safe (like the above).
> > > I attach the patch here to show what I mean by insecure
> > > worker->cp->addr usage if we start to modify it concurrently, though
> > > more work is needed it seems..
> > >
> > > If I am correct we need to refcount the worker->cp->addr (or rather a
> > > worker->address struct). I had a patch which does that to handle a
> > > proxy address_ttl parameter (above which address is renewed), I can
> > > try to revive and mix it with the attached one, in a PR. WDYT?
> >
> > Thanks for the good catch. With regards to mod_proxy_ajp I think we should
> > use conn->addr instead of worker->cp->addr. We cannot be sure that worker->cp->addr is really set.
> > I guess it did not cause any problems so far as in the AJP case we always reuse connections by default
> > and hence worker->cp->addr is set.
> >
> > I think using a refcount could be burdensome as we need to ensure correct increase and decrease of the refcount and
> > we might have 3rd party modules that do not play well. Hence I propose a copy approach (but thinking of it, it might
> > fail in the same or other ways with 3rd party modules using worker->cp->addr directly in a similar way like mod_proxy_ajp does today)
>
> Let me look at what I can come up with and compare with what you
> propose below (which does not look trivial either from a quick look).
> That is to say, I need a bit more time to have an opinion here :)
So I went with https://github.com/apache/httpd/pull/367 (sorry, not
much detailed commits yet), and specifically the first commit for the
reuse / ttl / force-expiring of the worker address(es).
The new function is (named after ap_proxy_determine_connection):
/*
* Resolve an address, reusing the one of the worker if any.
* @param proxy_function calling proxy scheme (http, ajp, ...)
* @param conn proxy connection the address is used for
* @param hostname host to resolve (should be the worker's if reusable)
* @param hostport port to resolve (should be the worker's if reusable)
* @param r current request (if any)
* @param s current server (or NULL if r != NULL and ap_proxyerror()
* should be called on error)
* @return APR_SUCCESS or an error
*/
PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char
*proxy_function,
proxy_conn_rec *conn,
const char *hostname,
apr_port_t hostport,
request_rec *r,
server_rec *s);
and it takes a proxy_conn_rec only, which simplifies things and which
we can provide everywhere after all (see the changes in mod_proxy_ftp
and mod_proxy_hcheck).
A new struct proxy_address (refcounted, TTL'ed) is defined to store an
address (when needed only, i.e. worker->is_address_reusable). It's
allocated as a subpool of worker->cp->dns_pool and looks like this:
struct proxy_address {
apr_sockaddr_t *addr; /* Remote address info */
const char *hostname; /* Remote host name */
apr_port_t hostport; /* Remote host port */
apr_uint32_t refcount; /* Number of conns and/or worker using it */
apr_uint32_t expiry; /* Expiry timestamp (seconds to
proxy_start_time) */
};
Each "reusable" proxy_worker and proxy_conn_rec owns a refcount to its
current address such that the last owner destroys the old one when
replacing it with the new one, which all happens in
ap_proxy_determine_address().
There are 2 ways an address can expire, either its TTL is over
(configured in the worker's addressTTL= parameter), or by forcing
(conn->)address->expiry = 0 (e.g. when ap_proxy_connect_backend()
tries to reconnect with a new DNS lookup, per your changes in
r1909451). There is a proxy_address_set_expired() helper for the
latter case to synchronize and avoid lookup storms.
The address expiry time is shared between all the children processes
in worker->s->address_expiry, even though there is a "copy" in each
address->expiry field to allow for forcing the DNS lookup and
synchronizing per child.
Because I didn't want to introduce the first use of the apr_atomic_*64
API in httpd (not sure which first APR version introduced it and only
the latest versions are reliable anyway), the expiry is 32bit
timestamp in seconds relative to some new proxy_start_time epoch
(initialized at httpd startup). The timestamps are taken from
apr_time_now() which is not a monotonic clock so it's not ideal (don't
change your system's time while httpd is running!), I think we need an
ap_monotonic_now() but that's another story (which I drafted in PR
#294 for mpm_event FWIW).
Since conn->addr and conn->hostname (i.e. conn->address->*) always
point to some valid memory (can be NULL before
ap_proxy_determine_address() still), they can be used safely during
connection processing regardless of worker->address changing
underneath, the eventual change will be taken into account in the
proxy_conn_rec the next time it's reused (closing the "old" socket if
any).
Regarding worker->s->addr, it's now legacy and set by
ap_proxy_determine_address() only if it's NULL (that is at startup or
if the user explicitly NULLed it). The first address' refcount is set
to 2 such that worker->s->addr is never deallocated, users can
continue to use it although it will not necessarily be the latest/real
address in use. That way we do not break users, and they can switch to
using conn->addr instead if needed.
Note that *worker->address is never safe to use outside a
PROXY_THREAD_[UN]LOCK() critical section, but changing *conn->address
is and does the same if it's the current address, which is useful for
the safety/correctness (hopefully) of ap_proxy_determine_address()
BTW.
There are other changes in the first commit for the lifetime of the
conn->uds_path and conn->forward, but the other commits are not really
related (I happened to make these changes while walking the code..).
WDYT?
Regards;
Yann.
Re: svn commit: r1909401 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Jun 30, 2023 at 2:35 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 6/29/23 5:08 PM, Yann Ylavic wrote:
> > On Tue, Apr 25, 2023 at 1:57 PM <rp...@apache.org> wrote:
> >>
> >> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> >> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Apr 25 11:57:22 2023
> >> @@ -2790,7 +2790,11 @@ ap_proxy_determine_connection(apr_pool_t
> >> * The single DNS lookup is used once per worker.
> >> * If dynamic change is needed then set the addr to NULL
> >> * inside dynamic config to force the lookup.
> >> + *
> >> + * Clear the dns_pool before to avoid a memory leak in case
> >> + * we did the lookup again.
> >> */
> >> + apr_pool_clear(worker->cp->dns_pool);
> >> err = apr_sockaddr_info_get(&addr,
> >> conn->hostname, APR_UNSPEC,
> >> conn->port, 0,
> >
> > I'm not sure we can clear the dns_pool, some conn->addr allocated on
> > it may be still in use by other threads.
> > While reviewing your backport proposal, I noticed that
> > worker->cp->addr was used concurrently in several places in mod_proxy
> > with no particular care, so I started to code a follow up, but it
> > needs that apr_pool_clear() too somewhere to avoid the leak and
> > figured it may not be safe (like the above).
> > I attach the patch here to show what I mean by insecure
> > worker->cp->addr usage if we start to modify it concurrently, though
> > more work is needed it seems..
> >
> > If I am correct we need to refcount the worker->cp->addr (or rather a
> > worker->address struct). I had a patch which does that to handle a
> > proxy address_ttl parameter (above which address is renewed), I can
> > try to revive and mix it with the attached one, in a PR. WDYT?
>
> Thanks for the good catch. With regards to mod_proxy_ajp I think we should
> use conn->addr instead of worker->cp->addr. We cannot be sure that worker->cp->addr is really set.
> I guess it did not cause any problems so far as in the AJP case we always reuse connections by default
> and hence worker->cp->addr is set.
>
> I think using a refcount could be burdensome as we need to ensure correct increase and decrease of the refcount and
> we might have 3rd party modules that do not play well. Hence I propose a copy approach (but thinking of it, it might
> fail in the same or other ways with 3rd party modules using worker->cp->addr directly in a similar way like mod_proxy_ajp does today)
Let me look at what I can come up with and compare with what you
propose below (which does not look trivial either from a quick look).
That is to say, I need a bit more time to have an opinion here :)
>
> I modified some code from your patch especially ap_proxy_worker_addr_get:
Thanks;
Yann.
Re: svn commit: r1909401 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 6/29/23 5:08 PM, Yann Ylavic wrote:
> On Tue, Apr 25, 2023 at 1:57 PM <rp...@apache.org> wrote:
>>
>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Apr 25 11:57:22 2023
>> @@ -2790,7 +2790,11 @@ ap_proxy_determine_connection(apr_pool_t
>> * The single DNS lookup is used once per worker.
>> * If dynamic change is needed then set the addr to NULL
>> * inside dynamic config to force the lookup.
>> + *
>> + * Clear the dns_pool before to avoid a memory leak in case
>> + * we did the lookup again.
>> */
>> + apr_pool_clear(worker->cp->dns_pool);
>> err = apr_sockaddr_info_get(&addr,
>> conn->hostname, APR_UNSPEC,
>> conn->port, 0,
>
> I'm not sure we can clear the dns_pool, some conn->addr allocated on
> it may be still in use by other threads.
> While reviewing your backport proposal, I noticed that
> worker->cp->addr was used concurrently in several places in mod_proxy
> with no particular care, so I started to code a follow up, but it
> needs that apr_pool_clear() too somewhere to avoid the leak and
> figured it may not be safe (like the above).
> I attach the patch here to show what I mean by insecure
> worker->cp->addr usage if we start to modify it concurrently, though
> more work is needed it seems..
>
> If I am correct we need to refcount the worker->cp->addr (or rather a
> worker->address struct). I had a patch which does that to handle a
> proxy address_ttl parameter (above which address is renewed), I can
> try to revive and mix it with the attached one, in a PR. WDYT?
Thanks for the good catch. With regards to mod_proxy_ajp I think we should
use conn->addr instead of worker->cp->addr. We cannot be sure that worker->cp->addr is really set.
I guess it did not cause any problems so far as in the AJP case we always reuse connections by default
and hence worker->cp->addr is set.
I think using a refcount could be burdensome as we need to ensure correct increase and decrease of the refcount and
we might have 3rd party modules that do not play well. Hence I propose a copy approach (but thinking of it, it might
fail in the same or other ways with 3rd party modules using worker->cp->addr directly in a similar way like mod_proxy_ajp does today)
I modified some code from your patch especially ap_proxy_worker_addr_get:
Notes:
1. I am aware that apr_sockaddr_info_copy is only available in APR >= 1.6
2. ttlexpired is a pseudocode boolean to incorporate your idea of a TTL. Details would need to be worked out.
3. If *addrp != NULL the idea is that we have an acceptable apr_sockaddr_t provided that neither the TTL expired
or worker->cp->addr is NULL and thus we would need to do another lookup.
PROXY_DECLARE(apr_status_t) ap_proxy_worker_addr_get(proxy_worker *worker,
apr_sockaddr_t **addrp,
const char *host,
apr_port_t port,
request_rec *r,
server_rec *s,
proxy_conn_rec *conn, /* Can be NULL */
apr_pool_t *p)
{
apr_sockaddr_t *addr = NULL;
apr_status_t status = APR_SUCCESS;
int addr_reusable = (worker->s->is_address_reusable
&& !worker->s->disablereuse);
#if APR_HAS_THREADS
int worker_locked = 0;
#endif
apr_status_t rv;
if (conn) {
p = conn->pool;
addrp = &(conn->addr);
}
if (addr_reusable) {
addr = AP_VOLATILIZE_T(apr_sockaddr_t *, worker->cp->addr);
if (addr && !ttlexpired && *addrp) {
return APR_SUCCESS;
}
if (!addr || ttlexpired) {
#if APR_HAS_THREADS
if ((rv = PROXY_THREAD_LOCK(worker))) {
if (r) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO()
"proxy lock");
}
else {
ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, APLOGNO()
"proxy lock");
}
*addrp = NULL;
return rv;
}
/* Reload under the lock (might have been resolved in the meantime) */
addr = AP_VOLATILIZE_T(apr_sockaddr_t *, worker->cp->addr);
worker_locked = 1;
#endif
}
}
*addrp = NULL;
/* Need a DNS lookup? */
if (!addr || ttlexpired) {
apr_pool_t *lookup_pool;
if (conn) {
/*
* If we need to do a lookup we should do a fresh connect afterwards.
* Hence cleanup conn from scratch.
*/
apr_pool_clear(p);
conn = apr_pcalloc(p, sizeof(proxy_conn_rec));
conn->pool = p;
conn->worker = worker;
apr_pool_create(&(conn->scpool), p);
apr_pool_tag(conn->scpool, "proxy_conn_scpool");
}
if (addr_reusable) {
lookup_pool = worker->cp->dns_pool;
apr_pool_clear(lookup_pool);
}
else {
lookup_pool = p;
}
status = apr_sockaddr_info_get(&addr, host, APR_UNSPEC, port, 0, lookup_pool);
if (addr_reusable && status == APR_SUCCESS) {
AP_VOLATILIZE_T(apr_sockaddr_t *, worker->cp->addr) = addr;
}
}
if (status == APR_SUCCESS) {
if (addr_reusable) {
apr_sockaddr_info_copy(addrp, addr, p);
}
else {
*addrp = addr;
}
}
#if APR_HAS_THREADS
if (worker_locked && (rv = PROXY_THREAD_UNLOCK(worker))) {
if (r) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO()
"proxy unlock");
}
else {
ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, APLOGNO()
"proxy unlock");
}
return rv;
}
#endif
return status;
}
In ap_proxy_determine_connection we could then call
err = ap_proxy_worker_addr_get(worker, NULL,
conn->hostname, conn->port,
r, r->server, conn, NULL);
BTW: I think p was the wrong pool in your and conn->pool needs to be used instead.
In mod_proxy_hcheck.c we could use
ap_proxy_worker_addr_get(worker, addr,
worker->s->hostname_ex, worker->s->port,
NULL, ctx->s, NULL, p);
There are still some gory details to be worked out, but this would be my high level approach.
Regards
Rüdiger