Simplified and improved sendfile() code on Linux.

The ngx_linux_sendfile() function is now used for both normal sendfile()
and sendfile in threads.  The ngx_linux_sendfile_thread() function was
modified to use the same interface as ngx_linux_sendfile(), and is simply
called from ngx_linux_sendfile() when threads are enabled.

Special return code NGX_DONE is used to indicate that a thread task was
posted and no further actions are needed.

If number of bytes sent is less that what we were sending, we now always
retry sending.  This is needed for sendfile() in threads as the number
of bytes we are sending might have been changed since the thread task
was posted.  And this is also needed for Linux 4.3+, as sendfile() might
be interrupted at any time and provides no indication if it was interrupted
or not (ticket #1174).

1 files changed, 47 insertions, 67 deletions

diff --git a/src/os/unix/ngx_linux_sendfile_chain.c b/src/os/unix/ngx_linux_sendfile_chain.c
index 3c0696a02..b44724cdd 100644
--- a/src/os/unix/ngx_linux_sendfile_chain.c
+++ b/src/os/unix/ngx_linux_sendfile_chain.c
@@ -20,8 +20,8 @@ static ssize_t ngx_linux_sendfile(ngx_connection_t *c, ngx_buf_t *file,
 #error sendfile64() is required!
 #endif
 
-static ngx_int_t ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file,
-    size_t size, size_t *sent);
+static ssize_t ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file,
+    size_t size);
 static void ngx_linux_sendfile_thread_handler(void *data, ngx_log_t *log);
 #endif
 
@@ -56,10 +56,6 @@ ngx_linux_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit)
     ngx_chain_t   *cl;
     ngx_iovec_t    header;
     struct iovec   headers[NGX_IOVS_PREALLOCATE];
-#if (NGX_THREADS)
-    ngx_int_t      rc;
-    ngx_uint_t     thread_handled, thread_complete;
-#endif
 
     wev = c->write;
 
@@ -82,10 +78,6 @@ ngx_linux_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit)
 
     for ( ;; ) {
         prev_send = send;
-#if (NGX_THREADS)
-        thread_handled = 0;
-        thread_complete = 0;
-#endif
 
         /* create the iovec and coalesce the neighbouring bufs */
 
@@ -179,38 +171,19 @@ ngx_linux_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit)
             }
 #endif
 
-#if (NGX_THREADS)
-            if (file->file->thread_handler) {
-                rc = ngx_linux_sendfile_thread(c, file, file_size, &sent);
-
-                switch (rc) {
-                case NGX_OK:
-                    thread_handled = 1;
-                    break;
-
-                case NGX_DONE:
-                    thread_complete = 1;
-                    break;
-
-                case NGX_AGAIN:
-                    break;
-
-                default: /* NGX_ERROR */
-                    return NGX_CHAIN_ERROR;
-                }
-
-            } else
-#endif
-            {
-                n = ngx_linux_sendfile(c, file, file_size);
+            n = ngx_linux_sendfile(c, file, file_size);
 
-                if (n == NGX_ERROR) {
-                    return NGX_CHAIN_ERROR;
-                }
+            if (n == NGX_ERROR) {
+                return NGX_CHAIN_ERROR;
+            }
 
-                sent = (n == NGX_AGAIN) ? 0 : n;
+            if (n == NGX_DONE) {
+                /* thread task posted */
+                return in;
             }
 
+            sent = (n == NGX_AGAIN) ? 0 : n;
+
         } else {
             n = ngx_writev(c, &header);
 
@@ -225,21 +198,27 @@ ngx_linux_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit)
 
         in = ngx_chain_update_sent(in, sent);
 
-        if ((size_t) (send - prev_send) != sent) {
-#if (NGX_THREADS)
-            if (thread_handled) {
-                return in;
-            }
-
-            if (thread_complete) {
-                send = prev_send + sent;
-                continue;
-            }
-#endif
+        if (n == NGX_AGAIN) {
             wev->ready = 0;
             return in;
         }
 
+        if ((size_t) (send - prev_send) != sent) {
+
+            /*
+             * sendfile() on Linux 4.3+ might be interrupted at any time,
+             * and provides no indication if it was interrupted or not,
+             * so we have to retry till an explicit EAGAIN
+             *
+             * sendfile() in threads can also report less bytes written
+             * than we are prepared to send now, since it was started in
+             * some point in the past, so we again have to retry
+             */
+
+            send = prev_send + sent;
+            continue;
+        }
+
         if (send >= limit || in == NULL) {
             return in;
         }
@@ -258,6 +237,14 @@ ngx_linux_sendfile(ngx_connection_t *c, ngx_buf_t *file, size_t size)
     ssize_t    n;
     ngx_err_t  err;
 
+#if (NGX_THREADS)
+
+    if (file->file->thread_handler) {
+        return ngx_linux_sendfile_thread(c, file, size);
+    }
+
+#endif
+
 #if (NGX_HAVE_SENDFILE64)
     offset = file->file_pos;
 #else
@@ -324,9 +311,8 @@ typedef struct {
 } ngx_linux_sendfile_ctx_t;
 
 
-static ngx_int_t
-ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file, size_t size,
-    size_t *sent)
+static ssize_t
+ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file, size_t size)
 {
     ngx_event_t               *wev;
     ngx_thread_task_t         *task;
@@ -356,10 +342,14 @@ ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file, size_t size,
         task->event.complete = 0;
 
         if (ctx->err == NGX_EAGAIN) {
-            *sent = 0;
+            /*
+             * if wev->complete is set, this means that a write event
+             * happened while we were waiting for the thread task, so
+             * we have to retry sending even on EAGAIN
+             */
 
             if (wev->complete) {
-                return NGX_DONE;
+                return 0;
             }
 
             return NGX_AGAIN;
@@ -384,13 +374,7 @@ ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file, size_t size,
             return NGX_ERROR;
         }
 
-        *sent = ctx->sent;
-
-        if (ctx->sent == ctx->size || wev->complete) {
-            return NGX_DONE;
-        }
-
-        return NGX_AGAIN;
+        return ctx->sent;
     }
 
     if (task->event.active && ctx->file == file) {
@@ -399,9 +383,7 @@ ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file, size_t size,
          * or multiple calls of the next body filter from a filter
          */
 
-        *sent = 0;
-
-        return NGX_OK;
+        return NGX_DONE;
     }
 
     ctx->file = file;
@@ -414,9 +396,7 @@ ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file, size_t size,
         return NGX_ERROR;
     }
 
-    *sent = 0;
-
-    return NGX_OK;
+    return NGX_DONE;
 }