[Bug 14195] New: EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Summary: EINTR in WS_select
           Product: Wine
           Version: CVS/GIT
          Platform: Other
        OS/Version: other
            Status: UNCONFIRMED
          Severity: major
          Priority: P2
         Component: winsock
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

I have an Win32 application written in Borland Delphi using Eureka tool
(http://www.eurekalog.com/)
working in Wine but with one error.
If exception occured in main thread,
WinAPI 'select' call in secondary thread interrupted with WSAINTR error code.
The short Delphi example is in the attached sample.tar.bz2.

WSAINTR result in Win32 select mean

"A blocking Windows Socket 1.1 call was canceled through
WSACancelBlockingCall."
(http://msdn.microsoft.com/en-us/library/ms740141(VS.85).aspx)

But

"The WSACancelBlockingCall function has been removed in compliance with the
Windows Sockets 2 specification, revision 2.2.0.
The function is not exported directly by WS2_32.DLL and Windows Sockets 2
applications should not use this function.
Windows Sockets 1.1 applications that call this function are still supported
through the WINSOCK.DLL and WSOCK32.DLL."
(http://msdn.microsoft.com/en-us/library/ms741547(VS.85).aspx)

Wine translates EINTR error code into WSAINTR error code.
But in Linux EINTR have a different meaning

"EINTR  A signal occurred before any requested event."
(man poll)

I propose the following patch to ignore the EINTR error code in Wine
'WS_select' based on Linux poll call:

diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c
index fa908b4..a98717e 100644
--- a/dlls/ws2_32/socket.c
+++ b/dlls/ws2_32/socket.c
@@ -2591,7 +2591,10 @@ int WINAPI WS_select(int nfds, WS_fd_set *ws_readfds,

if (ws_timeout) timeout = (ws_timeout->tv_sec * 1000) +
(ws_timeout->tv_usec + 999) / 1000;

-    ret = poll( pollfds, count, timeout );
+    while ((ret = poll( pollfds, count, timeout )) < 0)
+    {
+        if (errno != EINTR) break;
+    };
     release_poll_fds( ws_readfds, ws_writefds, ws_exceptfds, pollfds );

if (ret == -1) SetLastError(wsaErrno());

The wine already have a similar EINTR ignore

git-diff 84232099^ 84232099 -- dlls/winsock/socket.c
index cfbf49f..0f1ba34 100644
--- a/dlls/winsock/socket.c
+++ b/dlls/winsock/socket.c
@@ -589,7 +589,13 @@ static inline int do_block( int fd, int events )

pfd.fd = fd;
   pfd.events = events;
-  poll(&pfd, 1, -1);
+
+  while (poll(&pfd, 1, -1) < 0)
+  {
+      if (errno != EINTR)
+          return -1;
+  }
+
   return pfd.revents;
 }

Thank you for attension and for great software.
Best regards.

Dmitriy Anisimkov.

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #1 from Dmitriy Anisimkov   2008-06-29 23:46:07 ---
Created an attachment (id=14467)
 --> (http://bugs.winehq.org/attachment.cgi?id=14467)
Borland Delphi example

secondary thread WSAINTR error select call, when exception in main thread using
Eureka tool.

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #2 from Dmitriy Anisimkov   2008-06-30 02:33:23 ---
Created an attachment (id=14468)
 --> (http://bugs.winehq.org/attachment.cgi?id=14468)
EINTR in select

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Dmitriy Anisimkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #14467|Borland Delphi example      |ignore please this
        description|                            |attachemnt. See next
                   |                            |instead. I do not know how
                   |                            |to delete it from bug
                   |                            |description.
  Attachment #14467|sample.tar.bz2              |
           filename|                            |

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch, source
         OS/Version|other                       |Linux

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Damjan Jovanovic  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #3 from Damjan Jovanovic   2008-07-23 16:01:07 ---
Please send patches to the wine-devel mailing list, nobody looks at them here.

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|CVS/GIT                     |unspecified

--- Comment #4 from Austin English   2009-01-20 02:39:23 ---
Removing deprecated CVS/GIT version tag. Please retest in current git. If the
bug is still present in today's wine, but was not present in some earlier
version of wine, please update version field to earliest known version of wine
that had the bug. Thanks!

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #5 from Dmitriy Anisimkov   2009-01-20 03:14:15 ---
> Please send patches to the wine-devel mailing list, nobody looks at them here.

The patch to fix this bug supplied in October.

http://www.winehq.org/pipermail/wine-patches/2008-October/062652.html

Nobody get it for 3 monthes. Maybe I did something wrong ?

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #6 from Damjan Jovanovic   2009-01-20 04:46:20 ---
Post to the wine-devel mailing list asking for feedback on that patch. It looks
fine to me, but maybe a test case would make it more convincing.

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #7 from Alexandre Julliard   2009-01-20 05:20:36 ---
The patch is wrong, you can't just restart the poll, you need to update the
timeout.

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #8 from Dmitriy Anisimkov   2009-01-20 05:51:02 ---
(In reply to comment #7)
> The patch is wrong, you can't just restart the poll, you need to update the
> timeout.

Wine already have such ignorance without timeout recalculation.

static inline int do_block( int fd, int events, int timeout )
{
  struct pollfd pfd;
  int ret;

pfd.fd = fd;
  pfd.events = events;

while ((ret = poll(&pfd, 1, timeout)) < 0)
  {
      if (errno != EINTR)
          return -1;
  }
  if( ret == 0 )
      return 0;
  return pfd.revents;
}

I do not think that timeout recalculation is really necessary.
This situation very rare. If we do not recalculate timeout, we would have
just a bit longer timeout in very rare cases. The timeout is not so explicit
requierments for most cases. On timeout recalculation, we would pay by one more
system call to get system time before call "poll".

Do you really stand on timeout recalculation ?

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #9 from Alexandre Julliard   2009-01-20 06:13:12 ---
(In reply to comment #8)
> (In reply to comment #7)
> > The patch is wrong, you can't just restart the poll, you need to update the
> > timeout.
>
> Wine already have such ignorance without timeout recalculation.
>
> static inline int do_block( int fd, int events, int timeout )

Yes that one is wrong too, but it's always called with -1 so that's not a
problem in practice. Feel free to send a patch removing the timeout arg.
> I do not think that timeout recalculation is really necessary.
> This situation very rare. If we do not recalculate timeout, we would have
> just a bit longer timeout in very rare cases. The timeout is not so explicit
> requierments for most cases. On timeout recalculation, we would pay by one more
> system call to get system time before call "poll".
>
> Do you really stand on timeout recalculation ?

Yes of course. If we followed your reasoning we could also argue that EINTR
happening in that poll is a rare case and that we don't need to fix it. But as
this bug shows, rare cases do happen.

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

--- Comment #10 from Andrew Nguyen   2009-07-21 13:40:13 ---
With 1.1.26, I get:

Server: Started
Client: Started
Client: Client socket connected
Server: Client socket accepted
err:ntdll:RtlpWaitForCriticalSection section 0x451058 "?" wait timed out in
thread 0015, blocked by 0020, retrying (60 sec)

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download

--- Comment #11 from Austin English   2010-05-19 16:35:47 ---
Is this still an issue in current (1.1.44 or newer) wine?

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Andrew Nguyen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from Andrew Nguyen   2010-05-19 16:49:24 ---
f951deec6f48e815281f152a750f8ddbb604587a was committed back in October 2009,
and the test executable works as expected, so this was fixed for quite a long
time.

[Bug 14195] EINTR in WS_select

http://bugs.winehq.org/show_bug.cgi?id=14195

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #13 from Alexandre Julliard   2010-05-21 14:38:47 ---
Closing bugs fixed in 1.2-rc1.