On 6/19/24 2:04 PM, Yann Ylavic wrote:
> On Wed, Jun 19, 2024 at 1:00 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>
>> On 6/19/24 12:00 PM, Yann Ylavic wrote:
>>> On Wed, Jun 19, 2024 at 9:07 AM Ruediger Pluem <rpl...@apache.org> wrote:
>>>>
>>>> On 6/18/24 4:20 PM, yla...@apache.org wrote:
>>>>>
>>>>> + /* Release the old conn address */
>>>>> + if (conn->address) {
>>>>> + /* conn->address->addr cannot be released while it's
>>>>> used by
>>>>> + * conn->sock->remote_addr, thus if the old address is
>>>>> still
>>>>> + * the one used at apr_socket_connect() time AND the
>>>>> socket
>>>>> + * shouldn't be closed because the newly resolved
>>>>> address is
>>>>> + * the same. In this case let's (re)attach the old
>>>>> address to
>>>>> + * the socket lifetime (scpool), in any other case just
>>>>> release
>>>>> + * it now.
>>>>> + */
>>>>> + int addr_alive = 0,
>>>>> + conn_alive = (conn->sock && conn->addr &&
>>>>> + proxy_addrs_equal(conn->addr,
>>>>> address->addr));
>>>>> + if (conn_alive) {
>>>>> + apr_sockaddr_t *remote_addr = NULL;
>>>>> + apr_socket_addr_get(&remote_addr, APR_REMOTE,
>>>>> conn->sock);
>>>>> + addr_alive = (conn->addr == remote_addr);
>>>>
>>>> How can this ever be true? The remote_addr is always allocated from
>>>> conn->scpool. How can
>>>> conn->addr ever be allocated from conn->scpool? I think it always gets
>>>> allocated from
>>>> either a subpool of worker->cp->dns_pool, from conn->pool or from
>>>> conn->uds_pool.
>>>>
>>>>> + }
>>>
>>> All the complications[1] in this commit come from the fact that
>>> apr_socket_connect(sock, addr) does a simple pointer copy of addr into
>>> sock->remote_addr, which apr_socket_addr_get(APR_REMOTE) will return.
>>
>> As far as I read the code it does not.
>>
>> https://github.com/apache/apr/blob/b0a08c76ceacc2308d7cf1d5a7bb3c9b4665a432/network_io/unix/sockets.c#L423-L433
>>
>> We copy the data (sa, salen family and port) not a pointer.
>
> Ah yes, I was looking at win32 code, while Joe fixed it 13 years ago
> for unix (r983618).
And I was only looking at Unix :-)
> So the pointer copy exists, but only for WIN32 and OS/2 AFAICT, what a mess.
Indeed. What a mess.
>
> Let me fix that then ;)
Thanks. I guess these changes are backportable in APR, but how do we deal with
this in httpd?
Do we only follow the more complex code approach that you designed in case of
WIN32 and an old version of APR
and do a simpler approach on Unix and newer APR?
Regards
Rüdiger
