apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Saxon Druce" <sa...@blocksoftware.com>
Subject Re: [PATCH] WIN32 implementation of apr_poll_revents_get()
Date Tue, 04 Jun 2002 00:41:03 GMT
----- Original Message -----
From: <jfbriere@yahoo.com>
To: <dev@apr.apache.org>
Sent: Tuesday, June 04, 2002 6:43 AM
Subject: [PATCH] WIN32 implementation of apr_poll_revents_get()


>
> Consider this piece of code, where essentially a server poll() on its
socket,
> waiting for any client connections, then on any event received from poll,
> test for errors (eg. the server socket has been closed),
> and THEN do an accept() to get the client's connected socket:
>
> ...
>
> /* Here we wait indefinitely... */
> ret = apr_poll(poll, &n, -1);
> if (ret != APR_SUCCESS) return NULL;
>
> ret = apr_poll_revents_get(&event, sock_srvr, poll);
> if (ret != APR_SUCCESS) return NULL;
>
> /* Verify if there was any error */
> if (event & (APR_POLLERR | APR_POLLHUP | APR_POLLNVAL)) {
>     apr_socket_close(sock_srvr);
>     return NULL;
> }
>
> /* Ok there was a connection from a client */
> ret = apr_accept(&sock_client, sock_srvr, context);
> if (ret != APR_SUCCESS) return NULL;
>
> ...
>
> This will work on UNIX (I have tested it on SunOS 5.6, HPUX 11.00, AIX 4.2
and AIX 5.1)
> but it fails on WIN32.
> The reason is that in apr_poll_revents_get() function, WSARecv() returns
error 10057,
> which is WSAENOTCONN, which means: "The socket is not connected.".
> I trap this returned error code to make an additional test to see if there
is indeed an error.
>
> Here is the diff :
>
> [snip]

I submitted the following patch for the same problem about two months ago,
then I reposted it a month ago.

========================================================================
--- apr/network_io/win32/poll.c.old Mon Mar 25 16:37:46 2002
+++ apr/network_io/win32/poll.c Mon Mar 25 16:38:53 2002
@@ -156,6 +156,9 @@
     DWORD dummy;
     DWORD flags = MSG_PEEK;

+    BOOL optval;
+    int optlen;
+
     /* We just want to PEEK at the data, so I am setting up a dummy WSABUF
      * variable here.
      */
@@ -164,27 +167,45 @@

     if (FD_ISSET(sock->sock, aprset->read)) {
         revents |= APR_POLLIN;
-        if (WSARecv(sock->sock, &data, 1, &dummy, &flags, NULL,
-                    NULL) == SOCKET_ERROR) {
-            /* This is only legit since we don't return the error */
-            dummy = WSAGetLastError();
-            switch (dummy) {
-                case WSAECONNRESET:
-                case WSAECONNABORTED:
-                case WSAESHUTDOWN:
-                case WSAENETRESET: {
-                    revents ^= APR_POLLIN;
-                    revents |= APR_POLLHUP;
-                    break;
-                }
-                case WSAENOTSOCK: {
-                    revents ^= APR_POLLIN;
-                    revents |= APR_POLLNVAL;
-                }
-                default: {
-                    revents ^= APR_POLLIN;
-                    revents |= APR_POLLERR;
+
+        /* Check if the socket is listening for a connection (SO_ACCEPTCONN
+         * is true) - if it is we just leave APR_POLLIN in revents and
+         * don't attempt the WSARecv, since it would return WSAENOTCONN.
+         */
+        optlen = sizeof(optval);
+        getsockopt(sock->sock, SOL_SOCKET, SO_ACCEPTCONN, (char *)&optval,
+            &optlen);
+        if (!optval) {
+            /* The socket is not listening */
+            if (WSARecv(sock->sock, &data, 1, &dummy, &flags, NULL,
+                        NULL) == SOCKET_ERROR) {
+                /* This is only legit since we don't return the error */
+                dummy = WSAGetLastError();
+                switch (dummy) {
+                    case WSAECONNRESET:
+                    case WSAECONNABORTED:
+                    case WSAESHUTDOWN:
+                    case WSAENETRESET: {
+                        revents ^= APR_POLLIN;
+                        revents |= APR_POLLHUP;
+                        break;
+                    }
+                    case WSAENOTSOCK: {
+                        revents ^= APR_POLLIN;
+                        revents |= APR_POLLNVAL;
+                    }
+                    default: {
+                        revents ^= APR_POLLIN;
+                        revents |= APR_POLLERR;
+                    }
                 }
+            }
+            else if ((dummy == 0) && (sock->type == SOCK_STREAM)) {
+                /* Zero bytes received (dummy is the number of bytes)
+                 * from a stream socket indicates graceful shutdown.
+                 */
+                revents ^= APR_POLLIN;
+                revents |= APR_POLLHUP;
             }
         }
     }

========================================================================

If it hasn't been committed because there's something wrong with it, I'd be
happy to write it a different way. Eg it can be done without a call to
getsockopt() by adding a 'connected' flag to the apr_socket_t structure,
just like the unix version does if HAVE_POLL is false.

cya,
Saxon Druce (saxon@blocksoftware.com)



Mime
View raw message