On Fri, 9 Aug 2024 at 08:29, Ruediger Pluem <rpl...@apache.org> wrote:
> 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? > > Which patch do you mean r1918412 or something else? I didn't dig into details, but non-blocking calls on Windows may complete synchronously. > 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. > >> > >> > > > -- Ivan Zhakov
