You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@gmail.com> on 2011/04/13 16:53:33 UTC

Re: svn commit: r1089031 - in /apr/apr/trunk: CHANGES network_io/win32/sockets.c

On Tue, Apr 5, 2011 at 9:28 AM,  <tr...@apache.org> wrote:
> Author: trawick
> Date: Tue Apr  5 13:28:59 2011
> New Revision: 1089031
>
> URL: http://svn.apache.org/viewvc?rev=1089031&view=rev
> Log:
> restructure Windows apr_socket_connect() to more closely match
> the Unix implementation, fixing the getpeername() optimization
> and WSAEISCONN behavior
>
> PR: 48736 (covered WSAISCONN issue)
> Submitted by: <inoue ariel-networks.com> (WSAISCONN handling)
> Reworked/extended by: trawick

trivia: getpeername() fails for a connected IPv6 socket on XP; this
change to get the getpeername() optimization to work "fixes" that for
the common case

I'll probably backport this to 1.4.x for other reasons (but after 1.4.3).

Re: svn commit: r1089031 - in /apr/apr/trunk: CHANGES network_io/win32/sockets.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Apr 13, 2011 at 2:53 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Wed, Apr 13, 2011 at 2:28 PM, William A. Rowe Jr.
> <wr...@rowe-clan.net> wrote:
>> On 4/13/2011 1:11 PM, Jeff Trawick wrote:
>>> On Wed, Apr 13, 2011 at 1:51 PM, William A. Rowe Jr.
>>> <wr...@rowe-clan.net> wrote:
>>>> On 4/13/2011 9:53 AM, Jeff Trawick wrote:
>>>>> On Tue, Apr 5, 2011 at 9:28 AM,  <tr...@apache.org> wrote:
>>>>>> Author: trawick
>>>>>> Date: Tue Apr  5 13:28:59 2011
>>>>>> New Revision: 1089031
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1089031&view=rev
>>>>>> Log:
>>>>>> restructure Windows apr_socket_connect() to more closely match
>>>>>> the Unix implementation, fixing the getpeername() optimization
>>>>>> and WSAEISCONN behavior
>>>>>>
>>>>>> PR: 48736 (covered WSAISCONN issue)
>>>>>> Submitted by: <inoue ariel-networks.com> (WSAISCONN handling)
>>>>>> Reworked/extended by: trawick
>>>>>
>>>>> trivia: getpeername() fails for a connected IPv6 socket on XP; this
>>>>> change to get the getpeername() optimization to work "fixes" that for
>>>>> the common case
>>>>
>>>> Is this another aspect of the same flaw we just worked through, when I last
>>>> attempted to normalize this?  (That one broke IP addresses on win2k, see
>>>> for example https://issues.apache.org/bugzilla/show_bug.cgi?id=41693 ).
>>>
>>> That one is hard to follow...  It points to another bug, that one said
>>> Win32DisableAcceptEX was a work-around, and there's a later update
>>> from you saying "solved after 2.2.4 for windows 2000".
>>>
>>> I'll look a little further for earlier bug fixes.
>>>
>>>>> I'll probably backport this to 1.4.x for other reasons (but after 1.4.3).
>>>>
>>>> So if we tag httpd-2.3-something against 1.4.3, with IPv6 enabled, we will
>>>> start collecting similar reports to the one above, for XP IPv6 connections?
>>>>
>>>
>>> In testing 1.4.latest on XP this a.m. I didn't see an issue with my
>>> simple server app which retrieved local and remote addresses, as the
>>> getpeername() optimization on that side of the connection is working.
>>>
>>> apr_socket_accept() has this line to let it use the info from accept()
>>> instead of calling getpeername():     (*new)->remote_addr_unknown = 0;
>>>
>>> With 1.4.latest, the getpeername() optimization on the client side is
>>> not working, and getpeername() on IPv6 is where the XP bug is.
>>
>> What I'm suggesting is that AcceptEx fundamentally alters the way that
>> getpeername() et al will access the socket, sometimes to their detriment.
>>
>> Make sure you are testing both with and without using AcceptEx.

Using the latest httpd 2.2.x and apr 1.4.x on Windows 7, along with a
little debugging technique I like to call "printf", I can confirm that
neither getsockname() nor getpeername() are called by APR for
retrieving info about the connected socket, whether or not
Win32DisableAcceptEx is present, whether or not IPv6 sockets are used.
 Client addresses in the access and error log are formatted as
expected.

Avoiding getpeername() on this socket bypasses the IPv6-specific issue
on XP and the general issue on Win2K.

It would be cool to see it run there, but the code isn't going to
behave any differently w.r.t. getsockname()/getpeername().

Re: svn commit: r1089031 - in /apr/apr/trunk: CHANGES network_io/win32/sockets.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Apr 13, 2011 at 2:28 PM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On 4/13/2011 1:11 PM, Jeff Trawick wrote:
>> On Wed, Apr 13, 2011 at 1:51 PM, William A. Rowe Jr.
>> <wr...@rowe-clan.net> wrote:
>>> On 4/13/2011 9:53 AM, Jeff Trawick wrote:
>>>> On Tue, Apr 5, 2011 at 9:28 AM,  <tr...@apache.org> wrote:
>>>>> Author: trawick
>>>>> Date: Tue Apr  5 13:28:59 2011
>>>>> New Revision: 1089031
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1089031&view=rev
>>>>> Log:
>>>>> restructure Windows apr_socket_connect() to more closely match
>>>>> the Unix implementation, fixing the getpeername() optimization
>>>>> and WSAEISCONN behavior
>>>>>
>>>>> PR: 48736 (covered WSAISCONN issue)
>>>>> Submitted by: <inoue ariel-networks.com> (WSAISCONN handling)
>>>>> Reworked/extended by: trawick
>>>>
>>>> trivia: getpeername() fails for a connected IPv6 socket on XP; this
>>>> change to get the getpeername() optimization to work "fixes" that for
>>>> the common case
>>>
>>> Is this another aspect of the same flaw we just worked through, when I last
>>> attempted to normalize this?  (That one broke IP addresses on win2k, see
>>> for example https://issues.apache.org/bugzilla/show_bug.cgi?id=41693 ).
>>
>> That one is hard to follow...  It points to another bug, that one said
>> Win32DisableAcceptEX was a work-around, and there's a later update
>> from you saying "solved after 2.2.4 for windows 2000".
>>
>> I'll look a little further for earlier bug fixes.
>>
>>>> I'll probably backport this to 1.4.x for other reasons (but after 1.4.3).
>>>
>>> So if we tag httpd-2.3-something against 1.4.3, with IPv6 enabled, we will
>>> start collecting similar reports to the one above, for XP IPv6 connections?
>>>
>>
>> In testing 1.4.latest on XP this a.m. I didn't see an issue with my
>> simple server app which retrieved local and remote addresses, as the
>> getpeername() optimization on that side of the connection is working.
>>
>> apr_socket_accept() has this line to let it use the info from accept()
>> instead of calling getpeername():     (*new)->remote_addr_unknown = 0;
>>
>> With 1.4.latest, the getpeername() optimization on the client side is
>> not working, and getpeername() on IPv6 is where the XP bug is.
>
> What I'm suggesting is that AcceptEx fundamentally alters the way that
> getpeername() et al will access the socket, sometimes to their detriment.
>
> Make sure you are testing both with and without using AcceptEx.

I think the AcceptEx concern is the just path through APR. Please confirm:

WinNT MPM: AcceptEx + GetAcceptExSockaddrs() + apr_os_sock_make()

apr_os_sock_make() still sets remote_addr_unknown in alloc_socket(),
but since WinNT MPM passes in the remote socket address
remote_addr_unknown will be cleared.

(from previous e-mail)

the WinNT AcceptEx breakage was introduced when code was added to set
remote_addr_unknown for all sockets in alloc_socket(); as it was never
cleared, getpeername() would always be called; as getpeername() is
broken on Win2K, havoc ensued

the WinNT AcceptEx breakage was resolved when code was added to clear
remote_addr_unknown if apr_os_sock_make() is called with the remote
addr filled in

Re: svn commit: r1089031 - in /apr/apr/trunk: CHANGES network_io/win32/sockets.c

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 4/13/2011 1:11 PM, Jeff Trawick wrote:
> On Wed, Apr 13, 2011 at 1:51 PM, William A. Rowe Jr.
> <wr...@rowe-clan.net> wrote:
>> On 4/13/2011 9:53 AM, Jeff Trawick wrote:
>>> On Tue, Apr 5, 2011 at 9:28 AM,  <tr...@apache.org> wrote:
>>>> Author: trawick
>>>> Date: Tue Apr  5 13:28:59 2011
>>>> New Revision: 1089031
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1089031&view=rev
>>>> Log:
>>>> restructure Windows apr_socket_connect() to more closely match
>>>> the Unix implementation, fixing the getpeername() optimization
>>>> and WSAEISCONN behavior
>>>>
>>>> PR: 48736 (covered WSAISCONN issue)
>>>> Submitted by: <inoue ariel-networks.com> (WSAISCONN handling)
>>>> Reworked/extended by: trawick
>>>
>>> trivia: getpeername() fails for a connected IPv6 socket on XP; this
>>> change to get the getpeername() optimization to work "fixes" that for
>>> the common case
>>
>> Is this another aspect of the same flaw we just worked through, when I last
>> attempted to normalize this?  (That one broke IP addresses on win2k, see
>> for example https://issues.apache.org/bugzilla/show_bug.cgi?id=41693 ).
> 
> That one is hard to follow...  It points to another bug, that one said
> Win32DisableAcceptEX was a work-around, and there's a later update
> from you saying "solved after 2.2.4 for windows 2000".
> 
> I'll look a little further for earlier bug fixes.
> 
>>> I'll probably backport this to 1.4.x for other reasons (but after 1.4.3).
>>
>> So if we tag httpd-2.3-something against 1.4.3, with IPv6 enabled, we will
>> start collecting similar reports to the one above, for XP IPv6 connections?
>>
> 
> In testing 1.4.latest on XP this a.m. I didn't see an issue with my
> simple server app which retrieved local and remote addresses, as the
> getpeername() optimization on that side of the connection is working.
> 
> apr_socket_accept() has this line to let it use the info from accept()
> instead of calling getpeername():     (*new)->remote_addr_unknown = 0;
> 
> With 1.4.latest, the getpeername() optimization on the client side is
> not working, and getpeername() on IPv6 is where the XP bug is.

What I'm suggesting is that AcceptEx fundamentally alters the way that
getpeername() et al will access the socket, sometimes to their detriment.

Make sure you are testing both with and without using AcceptEx.

Re: svn commit: r1089031 - in /apr/apr/trunk: CHANGES network_io/win32/sockets.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Apr 13, 2011 at 2:11 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Wed, Apr 13, 2011 at 1:51 PM, William A. Rowe Jr.
> <wr...@rowe-clan.net> wrote:
>> On 4/13/2011 9:53 AM, Jeff Trawick wrote:
>>> On Tue, Apr 5, 2011 at 9:28 AM,  <tr...@apache.org> wrote:
>>>> Author: trawick
>>>> Date: Tue Apr  5 13:28:59 2011
>>>> New Revision: 1089031
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1089031&view=rev
>>>> Log:
>>>> restructure Windows apr_socket_connect() to more closely match
>>>> the Unix implementation, fixing the getpeername() optimization
>>>> and WSAEISCONN behavior
>>>>
>>>> PR: 48736 (covered WSAISCONN issue)
>>>> Submitted by: <inoue ariel-networks.com> (WSAISCONN handling)
>>>> Reworked/extended by: trawick
>>>
>>> trivia: getpeername() fails for a connected IPv6 socket on XP; this
>>> change to get the getpeername() optimization to work "fixes" that for
>>> the common case
>>
>> Is this another aspect of the same flaw we just worked through, when I last
>> attempted to normalize this?  (That one broke IP addresses on win2k, see
>> for example https://issues.apache.org/bugzilla/show_bug.cgi?id=41693 ).
>
> That one is hard to follow...  It points to another bug, that one said
> Win32DisableAcceptEX was a work-around, and there's a later update
> from you saying "solved after 2.2.4 for windows 2000".
>
> I'll look a little further for earlier bug fixes.

in httpd 2.2.4, the only change in network_io/win32 was a new line
which turned on remote_addr_unknown in alloc_socket()

strictly speaking, that was correct; unfortunately there were no
places where remote_addr_unknown was cleared; so getpeername() could
be called when it hadn't been previously

that was http://svn.apache.org/viewvc?view=revision&revision=480212

the core PR you're referring to is
https://issues.apache.org/bugzilla/show_bug.cgi?id=41321 -
getpeername() busted on Win2K

then you made this change to os_sock_make to resolve the WinNT MPM
issue (when AcceptEX is called):

http://svn.apache.org/viewvc?view=revision&revision=494531



>
>>> I'll probably backport this to 1.4.x for other reasons (but after 1.4.3).
>>
>> So if we tag httpd-2.3-something against 1.4.3, with IPv6 enabled, we will
>> start collecting similar reports to the one above, for XP IPv6 connections?
>>
>
> In testing 1.4.latest on XP this a.m. I didn't see an issue with my
> simple server app which retrieved local and remote addresses, as the
> getpeername() optimization on that side of the connection is working.
>
> apr_socket_accept() has this line to let it use the info from accept()
> instead of calling getpeername():     (*new)->remote_addr_unknown = 0;

whoops, that was my fix from the last couple of weeks; good, but no
bearing on how past Win2K symptoms were resolved

>
> With 1.4.latest, the getpeername() optimization on the client side is
> not working, and getpeername() on IPv6 is where the XP bug is.
>

Re: svn commit: r1089031 - in /apr/apr/trunk: CHANGES network_io/win32/sockets.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Apr 13, 2011 at 1:51 PM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On 4/13/2011 9:53 AM, Jeff Trawick wrote:
>> On Tue, Apr 5, 2011 at 9:28 AM,  <tr...@apache.org> wrote:
>>> Author: trawick
>>> Date: Tue Apr  5 13:28:59 2011
>>> New Revision: 1089031
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1089031&view=rev
>>> Log:
>>> restructure Windows apr_socket_connect() to more closely match
>>> the Unix implementation, fixing the getpeername() optimization
>>> and WSAEISCONN behavior
>>>
>>> PR: 48736 (covered WSAISCONN issue)
>>> Submitted by: <inoue ariel-networks.com> (WSAISCONN handling)
>>> Reworked/extended by: trawick
>>
>> trivia: getpeername() fails for a connected IPv6 socket on XP; this
>> change to get the getpeername() optimization to work "fixes" that for
>> the common case
>
> Is this another aspect of the same flaw we just worked through, when I last
> attempted to normalize this?  (That one broke IP addresses on win2k, see
> for example https://issues.apache.org/bugzilla/show_bug.cgi?id=41693 ).

That one is hard to follow...  It points to another bug, that one said
Win32DisableAcceptEX was a work-around, and there's a later update
from you saying "solved after 2.2.4 for windows 2000".

I'll look a little further for earlier bug fixes.

>> I'll probably backport this to 1.4.x for other reasons (but after 1.4.3).
>
> So if we tag httpd-2.3-something against 1.4.3, with IPv6 enabled, we will
> start collecting similar reports to the one above, for XP IPv6 connections?
>

In testing 1.4.latest on XP this a.m. I didn't see an issue with my
simple server app which retrieved local and remote addresses, as the
getpeername() optimization on that side of the connection is working.

apr_socket_accept() has this line to let it use the info from accept()
instead of calling getpeername():     (*new)->remote_addr_unknown = 0;

With 1.4.latest, the getpeername() optimization on the client side is
not working, and getpeername() on IPv6 is where the XP bug is.

Re: svn commit: r1089031 - in /apr/apr/trunk: CHANGES network_io/win32/sockets.c

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 4/13/2011 9:53 AM, Jeff Trawick wrote:
> On Tue, Apr 5, 2011 at 9:28 AM,  <tr...@apache.org> wrote:
>> Author: trawick
>> Date: Tue Apr  5 13:28:59 2011
>> New Revision: 1089031
>>
>> URL: http://svn.apache.org/viewvc?rev=1089031&view=rev
>> Log:
>> restructure Windows apr_socket_connect() to more closely match
>> the Unix implementation, fixing the getpeername() optimization
>> and WSAEISCONN behavior
>>
>> PR: 48736 (covered WSAISCONN issue)
>> Submitted by: <inoue ariel-networks.com> (WSAISCONN handling)
>> Reworked/extended by: trawick
> 
> trivia: getpeername() fails for a connected IPv6 socket on XP; this
> change to get the getpeername() optimization to work "fixes" that for
> the common case

Is this another aspect of the same flaw we just worked through, when I last
attempted to normalize this?  (That one broke IP addresses on win2k, see
for example https://issues.apache.org/bugzilla/show_bug.cgi?id=41693 ).

> I'll probably backport this to 1.4.x for other reasons (but after 1.4.3).

So if we tag httpd-2.3-something against 1.4.3, with IPv6 enabled, we will
start collecting similar reports to the one above, for XP IPv6 connections?