Any APR windows guy on the below?

On Windows apr_socket_connect(cd, sa) returns APR_SUCCESS despite being non 
blocking.
This doesn't sound correct. Can someone have a look on the patch?

Regards

Rüdiger

On 7/1/24 2:22 PM, Ruediger Pluem wrote:
> Crossposting to dev@apr. Maybe some Windows folks want to feedback.
> 
> Regards
> 
> Rüdiger
> 
> On 6/20/24 12:12 PM, Ruediger Pluem wrote:
>>
>>
>> On 6/19/24 2:52 PM, Joe Orton wrote:
>>> On Wed, Jun 19, 2024 at 02:04:20PM +0200, Yann Ylavic wrote:
>>>> On Wed, Jun 19, 2024 at 1:00 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>>>> 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.
>>>
>>> Sorry!
>>>
>>>> Let me fix that then ;)
>>>
>>> There is a test in that commit, is it not catching the bug on non-Unix?
>>
>> As far as I can tell the test is never hit on Windows as 
>> apr_socket_connect(cd, sa) returns APR_SUCCESS
>> despite being non blocking.
>>
>> https://github.com/apache/apr/blob/b0a08c76ceacc2308d7cf1d5a7bb3c9b4665a432/test/testsock.c#L428-L435
>>
>> .\testall.exe -v testsock
>> testsock            : |Line 433: Cannot test if connect completes 
>> synchronously
>> SUCCESS
>> All tests passed.
>>
>> The following patch would fix this by causing the test to executed and of 
>> course to fail:
>>
>> Index: network_io/win32/sockets.c
>> ===================================================================
>> --- network_io/win32/sockets.c       (revision 1917061)
>> +++ network_io/win32/sockets.c       (working copy)
>> @@ -378,7 +378,7 @@
>>      }
>>
>>      if (rv == APR_FROM_OS_ERROR(WSAEWOULDBLOCK)) {
>> -        if (sock->timeout == 0) {
>> +        if (sock->timeout <= 0) {
>>              /* Tell the app that the connect is in progress...
>>               * Gotta play some games here.  connect on Unix will return
>>               * EINPROGRESS under the same circumstances that Windows
>>
>>
>>
>> But as Windows is not my native environment I would appreciate remote eyes 
>> if the fix is correct.
>>
>>
> 

Reply via email to