apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject Re: [PATCH] make socket timeouts work reasonably for connect()
Date Tue, 10 Jul 2001 13:53:46 GMT
This seems much more consistent with how the other APR networkio calls work, so +1

Bill


> (Unix at least; Win32 connect is a mess for non-blocking/timed-out
> sockets... it looks to me that we wait forever; I could be confused
> though :) )
> 
> concerns?
> 
> old behavior on Unix:
> 
> if timeout is set on socket before apr_connect() and connect takes a
> while (normal for TCP), apr_connect() returns EINPROGRESS and app must
> handle any timeout
> 
> new behavior on Unix:
> 
> if timeout is set on socket before apr_connect(), app will see
> APR_ETIMEUP if it takes longer than the timeout
> 
> Index: include/arch/unix/networkio.h
> ===================================================================
> RCS file: /home/cvspublic/apr/include/arch/unix/networkio.h,v
> retrieving revision 1.45
> diff -u -r1.45 networkio.h
> --- include/arch/unix/networkio.h 2001/07/07 22:48:09 1.45
> +++ include/arch/unix/networkio.h 2001/07/10 05:46:27
> @@ -159,6 +159,7 @@
>  
>  const char *apr_inet_ntop(int af, const void *src, char *dst, apr_size_t size);
>  int apr_inet_pton(int af, const char *src, void *dst);
> +apr_status_t apr_wait_for_io_or_timeout(apr_socket_t *sock, int for_read);
>  
>  #define apr_is_option_set(mask, option)  ((mask & option) ==option)
>  
> Index: network_io/unix/sendrecv.c
> ===================================================================
> RCS file: /home/cvspublic/apr/network_io/unix/sendrecv.c,v
> retrieving revision 1.65
> diff -u -r1.65 sendrecv.c
> --- network_io/unix/sendrecv.c 2001/05/03 22:38:00 1.65
> +++ network_io/unix/sendrecv.c 2001/07/10 05:46:29
> @@ -59,7 +59,7 @@
>  #include "fileio.h"
>  #endif /* APR_HAS_SENDFILE */
>  
> -static apr_status_t wait_for_io_or_timeout(apr_socket_t *sock, int for_read)
> +apr_status_t apr_wait_for_io_or_timeout(apr_socket_t *sock, int for_read)
>  {
>      struct timeval tv, *tvptr;
>      fd_set fdset;
> @@ -103,7 +103,7 @@
>  
>      if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) 
>        && sock->timeout != 0) {
> -        apr_status_t arv = wait_for_io_or_timeout(sock, 0);
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 0);
>          if (arv != APR_SUCCESS) {
>              *len = 0;
>              return arv;
> @@ -132,7 +132,7 @@
>  
>      if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) && 
>        sock->timeout != 0) {
> -        apr_status_t arv = wait_for_io_or_timeout(sock, 1);
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 1);
>          if (arv != APR_SUCCESS) {
>              *len = 0;
>              return arv;
> @@ -167,7 +167,7 @@
>  
>      if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK)
>          && sock->timeout != 0) {
> -        apr_status_t arv = wait_for_io_or_timeout(sock, 0);
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 0);
>          if (arv != APR_SUCCESS) {
>              *len = 0;
>              return arv;
> @@ -200,7 +200,7 @@
>  
>      if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) &&
>          sock->timeout != 0) {
> -        apr_status_t arv = wait_for_io_or_timeout(sock, 1);
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 1);
>          if (arv != APR_SUCCESS) {
>              *len = 0;
>              return arv;
> @@ -235,7 +235,7 @@
>  
>      if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) && 
>        sock->timeout != 0) {
> -        apr_status_t arv = wait_for_io_or_timeout(sock, 0);
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 0);
>          if (arv != APR_SUCCESS) {
>              *len = 0;
>              return arv;
> @@ -324,7 +324,7 @@
>      if (rv == -1 && 
>          (errno == EAGAIN || errno == EWOULDBLOCK) && 
>          sock->timeout > 0) {
> - arv = wait_for_io_or_timeout(sock, 0);
> + arv = apr_wait_for_io_or_timeout(sock, 0);
>   if (arv != APR_SUCCESS) {
>       *len = 0;
>       return arv;
> @@ -423,7 +423,7 @@
>           *      we get -1/EAGAIN/nbytes>0; AFAICT it just means extra syscalls
>           *      from time to time
>           */
> -        apr_status_t arv = wait_for_io_or_timeout(sock, 0);
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 0);
>          if (arv != APR_SUCCESS) {
>              *len = 0;
>              return arv;
> @@ -577,7 +577,7 @@
>      if (rc == -1 && 
>          (errno == EAGAIN || errno == EWOULDBLOCK) && 
>          sock->timeout > 0) {
> -        apr_status_t arv = wait_for_io_or_timeout(sock, 0);
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 0);
>  
>          if (arv != APR_SUCCESS) {
>              *len = 0;
> @@ -710,7 +710,7 @@
>      if (rv == -1 &&
>          (errno == EAGAIN || errno == EWOULDBLOCK) &&
>          sock->timeout > 0) {
> -        arv = wait_for_io_or_timeout(sock, 0);
> +        arv = apr_wait_for_io_or_timeout(sock, 0);
>          if (arv != APR_SUCCESS) {
>              *len = 0;
>              return arv;
> Index: network_io/unix/sockets.c
> ===================================================================
> RCS file: /home/cvspublic/apr/network_io/unix/sockets.c,v
> retrieving revision 1.74
> diff -u -r1.74 sockets.c
> --- network_io/unix/sockets.c 2001/05/02 02:32:47 1.74
> +++ network_io/unix/sockets.c 2001/07/10 05:46:29
> @@ -257,39 +257,57 @@
>  
>  apr_status_t apr_connect(apr_socket_t *sock, apr_sockaddr_t *sa)
>  {
> +    int rc;
> +
>      if ((sock->socketdes < 0) || (!sock->remote_addr)) {
>          return APR_ENOTSOCK;
>      }
> +
> +    do {
> +        rc = connect(sock->socketdes,
> +                     (const struct sockaddr *)&sa->sa.sin,
> +                     sa->salen);
> +    } while (rc == -1 && errno == EINTR);
> +
> +    if (rc == -1 && errno == EINPROGRESS && sock->timeout != 0) {
> +        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 0);
> +        if (arv != APR_SUCCESS) {
> +            return arv;
> +        }
> +        else {
> +            do {
> +                rc = connect(sock->socketdes,
> +                             (const struct sockaddr *)&sa->sa.sin,
> +                             sa->salen);
> +            } while (rc == -1 && errno == EINTR);
> +        }
> +    }
>  
> -    if ((connect(sock->socketdes,
> -                 (const struct sockaddr *)&sa->sa.sin,
> -                 sa->salen) < 0) &&
> -        (errno != EINPROGRESS)) {
> +    if (rc == -1) {
>          return errno;
>      }
> -    else {
> -        sock->remote_addr = sa;
> -        /* XXX IPv6 assumes sin_port and sin6_port at same offset */
> -        if (sock->local_addr->sa.sin.sin_port == 0) {
> -            /* connect() got us an ephemeral port */
> -            sock->local_port_unknown = 1;
> -        }
> -        /* XXX IPv6 to be handled better later... */
> -        if (
> +
> +    sock->remote_addr = sa;
> +    /* XXX IPv6 assumes sin_port and sin6_port at same offset */
> +    if (sock->local_addr->sa.sin.sin_port == 0) {
> +        /* connect() got us an ephemeral port */
> +        sock->local_port_unknown = 1;
> +    }
> +    /* XXX IPv6 to be handled better later... */
> +    if (
>  #if APR_HAVE_IPV6
> -            sock->local_addr->sa.sin.sin_family == APR_INET6 ||
> +        sock->local_addr->sa.sin.sin_family == APR_INET6 ||
>  #endif
> -            sock->local_addr->sa.sin.sin_addr.s_addr == 0) {
> -            /* not bound to specific local interface; connect() had to assign
> -             * one for the socket
> -             */
> -            sock->local_interface_unknown = 1;
> -        }
> +        sock->local_addr->sa.sin.sin_addr.s_addr == 0) {
> +        /* not bound to specific local interface; connect() had to assign
> +         * one for the socket
> +         */
> +        sock->local_interface_unknown = 1;
> +    }
>  #ifndef HAVE_POLL
> -        sock->connected=1;
> +    sock->connected=1;
>  #endif
> -        return APR_SUCCESS;
> -    }
> +    return APR_SUCCESS;
>  }
>  
>  apr_status_t apr_socket_data_get(void **data, const char *key, apr_socket_t *sock)
> Index: test/client.c
> ===================================================================
> RCS file: /home/cvspublic/apr/test/client.c,v
> retrieving revision 1.29
> diff -u -r1.29 client.c
> --- test/client.c 2001/06/08 04:49:43 1.29
> +++ test/client.c 2001/07/10 05:46:30
> @@ -72,7 +72,7 @@
>      char *local_ipaddr, *remote_ipaddr;
>      char *dest = "127.0.0.1";
>      apr_port_t local_port, remote_port;
> -    apr_interval_time_t read_timeout = 2 * APR_USEC_PER_SEC;
> +    apr_interval_time_t timeout = 2 * APR_USEC_PER_SEC;
>      apr_sockaddr_t *local_sa, *remote_sa;
>  
>      setbuf(stdout, NULL);
> @@ -81,7 +81,7 @@
>      }
>  
>      if (argc > 2) {
> -        read_timeout = APR_USEC_PER_SEC * atoi(argv[2]);
> +        timeout = atoi(argv[2]);
>      }
>  
>      fprintf(stdout, "Initializing.........");
> @@ -117,6 +117,14 @@
>      }
>      fprintf(stdout, "OK\n");
>  
> +    fprintf(stdout, "\tClient:  Setting socket timeout.......");
> +    stat = apr_setsocketopt(sock, APR_SO_TIMEOUT, timeout);
> +    if (stat) {
> +        fprintf(stderr, "Problem setting timeout: %d\n", stat);
> +        exit(-1);
> +    }
> +    fprintf(stdout, "OK\n");
> +
>      fprintf(stdout, "\tClient:  Connecting to socket.......");
>  
>      stat = apr_connect(sock, remote_sa);
> @@ -147,14 +155,6 @@
>      }
>      fprintf(stdout, "OK\n");
>     
> -    fprintf(stdout, "\tClient:  Setting read timeout.......");
> -    stat = apr_setsocketopt(sock, APR_SO_TIMEOUT, read_timeout);
> -    if (stat) {
> -        fprintf(stderr, "Problem setting timeout: %d\n", stat);
> -        exit(-1);
> -    }
> -    fprintf(stdout, "OK\n");
> -
>      length = STRLEN; 
>      fprintf(stdout, "\tClient:  Trying to receive data over socket.......");
>  
> 
> 
> -- 
> Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
>        http://www.geocities.com/SiliconValley/Park/9289/
>              Born in Roswell... married an alien...
> 


Mime
View raw message