Re: [PATCH] WIN32 implementation of apr_poll_revents_get()

[JF B]

What is exactly the problem with the getsockopt() function?

My implementation is slightly different from yours. I use the getsockopt() function only when WSAGetLastError() returns WSAENOTCONN.

By the way, why in your implementation don't you test the return of getsockopt()?

Also, I considered first using a flag in the apr_socket_t structure, but my problem is that there is already one: 'disconnected'. I would use this flag, but after many review of the code, I still don't know the utility of this flag.

Thanks!

Jean-Francois BRIERE

Saxon Druce <[EMAIL PROTECTED]> wrote:

----- Original Message -----
From: <[EMAIL PROTECTED]>
To:
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 ([EMAIL PROTECTED])

---

Do You Yahoo!?
Sign-up for Video Highlights of 2002 FIFA World Cup