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).
So the pointer copy exists, but only for WIN32 and OS/2 AFAICT, what a mess.
Let me fix that then ;)
Thanks;
Yann.
