Converting writing on socket to nonblocking io instead of SO_SNDTIMEO

diff --git a/libatalk/dsi/dsi_stream.c b/libatalk/dsi/dsi_stream.c
index 71d36953d0f207fe2ef04b2b3245cfbe6014245b..85caa5f59d45387970c6dcbc1c545e04edac24a3 100644 (file)
--- a/libatalk/dsi/dsi_stream.c
+++ b/libatalk/dsi/dsi_stream.c
@@ -72,66 +72,57 @@ static void dsi_init_buffer(DSI *dsi)
    afpd is sleeping too much while trying to send something.
    May be there's no reader or the reader is also sleeping in write,
    look if there's some data for us to read, hopefully it will wake up
-   the reader
+   the reader so we can write again.
 */
-static int dsi_buffer(DSI *dsi)
+static int dsi_peek(DSI *dsi)
 {
     fd_set readfds, writefds;
     int    len;
     int    maxfd;
+    int    ret;
 
-    LOG(log_maxdebug, logtype_dsi, "dsi_buffer: switching to non-blocking IO");
-
-    /* non blocking mode */
-    if (setnonblock(dsi->socket, 1) < 0) {
-        /* can't do it! exit without error it will sleep to death below */
-        LOG(log_error, logtype_dsi, "dsi_buffer: ioctl non blocking mode %s", strerror(errno));
-        return 0;
-    }
-    
     FD_ZERO(&readfds);
     FD_ZERO(&writefds);
     FD_SET( dsi->socket, &readfds);
     FD_SET( dsi->socket, &writefds);
     maxfd = dsi->socket +1;
+
     while (1) {
         FD_SET( dsi->socket, &readfds);
         FD_SET( dsi->socket, &writefds);
-        if (select( maxfd, &readfds, &writefds, NULL, NULL) <= 0)
-            break;
 
-        if ( !FD_ISSET(dsi->socket, &readfds)) {
-            /* nothing waiting in the read queue */
+        /* No timeout: if there's nothing to read nor nothing to write,
+         * we've got nothing to do at all */
+        if ((ret = select( maxfd, &readfds, &writefds, NULL, NULL)) <= 0) {
+            if (ret == -1 && errno == EINTR)
+                /* we might have been interrupted by out timer, so restart select */
+                continue;
+            /* give up */
             break;
         }
-        dsi_init_buffer(dsi);
-        len = dsi->end - dsi->eof;
-
-        if (len <= 0) {
-            /* ouch, our buffer is full ! 
-             * fall back to blocking IO 
-             * could block and disconnect but it's better than a cpu hog
-             */
-            break;
+
+        /* Check if there's sth to read, hopefully reading that will unblock the client */
+        if (FD_ISSET(dsi->socket, &readfds)) {
+            dsi_init_buffer(dsi);
+            len = dsi->end - dsi->eof;
+
+            if (len <= 0) {
+                /* ouch, our buffer is full ! fall back to blocking IO 
+                 * could block and disconnect but it's better than a cpu hog */
+                break;
+            }
+
+            len = read(dsi->socket, dsi->eof, len);
+            if (len <= 0)
+                break;
+            dsi->eof += len;
         }
 
-        len = read(dsi->socket, dsi->eof, len);
-        if (len <= 0)
-            break;
-        dsi->eof += len;
-        if ( FD_ISSET(dsi->socket, &writefds)) {
+        if (FD_ISSET(dsi->socket, &writefds))
             /* we can write again at last */
             break;
-        }
     }
 
-    LOG(log_maxdebug, logtype_dsi, "dsi_buffer: switching back to blocking IO");
-
-    if (setnonblock(dsi->socket, 0) < 0) {
-        /* can't do it! afpd will fail very quickly */
-        LOG(log_error, logtype_dsi, "dsi_buffer: ioctl blocking mode %s", strerror(errno));
-        return -1;
-    }
     return 0;
 }
 
@@ -143,51 +134,59 @@ ssize_t dsi_stream_write(DSI *dsi, void *data, const size_t length, int mode)
 {
   size_t written;
   ssize_t len;
-#if 0
-  /* FIXME sometime it's slower */
-  unsigned int flags = (mode)?MSG_MORE:0;
-#endif
   unsigned int flags = 0;
 
-#if 0
-  /* XXX there's no MSG_DONTWAIT in recv ?? so we have to play with ioctl
-  */ 
-  flags |= MSG_DONTWAIT;
-#endif
-  
   dsi->in_write++;
   written = 0;
 
   LOG(log_maxdebug, logtype_dsi, "dsi_stream_write: sending %u bytes", length);
 
+  /* non blocking mode */
+  if (setnonblock(dsi->socket, 1) < 0) {
+      LOG(log_error, logtype_dsi, "dsi_stream_write: setnonblock: %s", strerror(errno));
+      return -1;
+  }
+
   while (written < length) {
       len = send(dsi->socket, (u_int8_t *) data + written, length - written, flags);
-      if ((len == 0) || (len == -1 && errno == EINTR))
+      if (len >= 0) {
+          written += len;
+          continue;
+      }
+
+      if (errno == EINTR)
           continue;
 
-    if (len < 0) {
       if (errno == EAGAIN || errno == EWOULDBLOCK) {
           if (mode == DSI_NOWAIT && written == 0) {
               /* DSI_NOWAIT is used by attention give up in this case. */
-              return -1;
+              written = -1;
+              goto exit;
           }
-          if (dsi_buffer(dsi)) {
-              /* can't go back to blocking mode, exit, the next read
-                 will return with an error and afpd will die.
-              */
-              break;
+
+          /* Try to read sth. in order to break up possible deadlock */
+          if (dsi_peek(dsi) != 0) {
+              written = -1;
+              goto exit;
           }
+          /* Now try writing again */
           continue;
       }
+
       LOG(log_error, logtype_dsi, "dsi_stream_write: %s", strerror(errno));
-      break;
-    }
-    else {
-        written += len;
-    }
+      written = -1;
+      goto exit;
+  }
+
+  if (setnonblock(dsi->socket, 0) < 0) {
+      LOG(log_error, logtype_dsi, "dsi_stream_write: setnonblock: %s", strerror(errno));
+      written = -1;
+      goto exit;
   }
 
   dsi->write_count += written;
+
+exit:
   dsi->in_write--;
   return written;
 }
@@ -204,6 +203,12 @@ ssize_t dsi_stream_read_file(DSI *dsi, int fromfd, off_t offset, const size_t le
   dsi->in_write++;
   written = 0;
 
+  /* non blocking mode */
+  if (setnonblock(dsi->socket, 1) < 0) {
+      LOG(log_error, logtype_dsi, "dsi_stream_read_file: setnonblock: %s", strerror(errno));
+      return -1;
+  }
+
   while (written < length) {
     len = sys_sendfile(dsi->socket, fromfd, &offset, length - written);
         
@@ -214,7 +219,7 @@ ssize_t dsi_stream_read_file(DSI *dsi, int fromfd, off_t offset, const size_t le
           return -1;
           
       if (errno == EAGAIN || errno == EWOULDBLOCK) {
-          if (dsi_buffer(dsi)) {
+          if (dsi_peek(dsi)) {
               /* can't go back to blocking mode, exit, the next read
                  will return with an error and afpd will die.
               */
@@ -234,6 +239,11 @@ ssize_t dsi_stream_read_file(DSI *dsi, int fromfd, off_t offset, const size_t le
         written += len;
   }
 
+  if (setnonblock(dsi->socket, 0) < 0) {
+      LOG(log_error, logtype_dsi, "dsi_stream_read_file: setnonblock: %s", strerror(errno));
+      return -1;
+  }
+
   dsi->write_count += written;
   dsi->in_write--;
   return written;
@@ -411,7 +421,7 @@ int dsi_stream_send(DSI *dsi, void *buf, size_t length)
       break;
     else if (len < 0) { /* error */
       if (errno == EAGAIN || errno == EWOULDBLOCK) {
-          if (!dsi_buffer(dsi)) {
+          if (!dsi_peek(dsi)) {
               continue;
           }
       }

diff --git a/libatalk/dsi/dsi_tcp.c b/libatalk/dsi/dsi_tcp.c
index 74d92de1db91183e245a02ca857af8d7ebd4edb2..243003b5aac5c495a886fc0453161d9c377b49fd 100644 (file)
--- a/libatalk/dsi/dsi_tcp.c
+++ b/libatalk/dsi/dsi_tcp.c
@@ -86,25 +86,6 @@ static void dsi_tcp_close(DSI *dsi)
     dsi->socket = -1;
 }
 
-static void dsi_tcp_timeout(DSI *dsi)
-{
-    struct timeval tv;
-    /* 2 seconds delay, most of the time it translates to 4 seconds:
-     * send/write returns first with whatever it has written and the
-     * second time it returns EAGAIN
-     */
-    tv.tv_sec = 2;
-    tv.tv_usec = 0;
-
-    /* Note: write isn't a restartable syscall if there's a timeout on the socket
-     * we have to test for EINTR
-     */
-    if (setsockopt(dsi->socket, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)) < 0) {
-        LOG(log_error, logtype_dsi, "dsi_tcp_open: unable to set timeout %s", strerror(errno));
-        exit(EXITERR_CLNT);
-    }
-}
-
 /* alarm handler for tcp_open */
 static void timeout_handler(int sig _U_)
 {
@@ -223,8 +204,6 @@ static int dsi_tcp_open(DSI *dsi)
         sigaction(SIGALRM, &oldact, NULL);
 #endif
 
-        dsi_tcp_timeout(dsi);
-
         LOG(log_info, logtype_dsi, "AFP/TCP session from %s:%u",
             getip_string((struct sockaddr *)&dsi->client),
             getip_port((struct sockaddr *)&dsi->client));