[nginx] Mp4: fixed directio usage with open_file_cache.

[Maxim Dounin]

details:   http://freenginx.org/hg/nginx/rev/314192d67a91
branches:  
changeset: 9359:314192d67a91
user:      Maxim Dounin <mdounin at mdounin.ru>
date:      Sun May 25 23:57:28 2025 +0300
description:
Mp4: fixed directio usage with open_file_cache.

With open_file_cache, if directio is enabled on a file descriptor after
opening the file, other consumers won't know about directio being enabled
and will use unaligned reads, leading to EINVAL errors from pread()
on Linux.

Further, if a file descriptor with directio enabled will be returned to
the mp4 module itself, during mp4 file processing it will be used in
assumption that directio is not enabled.

Fix is to use of.directio and ngx_open_cached_file() to enable directio,
and switch it off during mp4 file processing.

While this does some unneeded syscalls if the file is actually just opened
and we have to parse the file, this shouldn't be significant compared to
other mp4 file processing costs.

diffstat:

 src/http/modules/ngx_http_mp4_module.c |  36 ++++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 14 deletions(-)

diffs (86 lines):

diff --git a/src/http/modules/ngx_http_mp4_module.c b/src/http/modules/ngx_http_mp4_module.c
--- a/src/http/modules/ngx_http_mp4_module.c
+++ b/src/http/modules/ngx_http_mp4_module.c
@@ -479,7 +479,7 @@ ngx_http_mp4_handler(ngx_http_request_t 
     u_char                    *last;
     size_t                     root;
     ngx_int_t                  rc, start, end;
-    ngx_uint_t                 level, length;
+    ngx_uint_t                 level, length, directio;
     ngx_str_t                  path, value;
     ngx_log_t                 *log;
     ngx_buf_t                 *b;
@@ -519,7 +519,7 @@ ngx_http_mp4_handler(ngx_http_request_t 
     ngx_memzero(&of, sizeof(ngx_open_file_info_t));
 
     of.read_ahead = clcf->read_ahead;
-    of.directio = NGX_MAX_OFF_T_VALUE;
+    of.directio = clcf->directio;
     of.valid = clcf->open_file_cache_valid;
     of.min_uses = clcf->open_file_cache_min_uses;
     of.errors = clcf->open_file_cache_errors;
@@ -579,6 +579,7 @@ ngx_http_mp4_handler(ngx_http_request_t 
 
     start = -1;
     length = 0;
+    directio = 0;
     r->headers_out.content_length_n = of.size;
     mp4 = NULL;
     b = NULL;
@@ -614,6 +615,21 @@ ngx_http_mp4_handler(ngx_http_request_t 
     if (start >= 0) {
         r->single_range = 1;
 
+        if (of.is_directio) {
+
+            /*
+             * DIRECTIO is set on transfer only
+             * to allow kernel to cache "moov" atom
+             */
+
+            if (ngx_directio_off(of.fd) == NGX_FILE_ERROR) {
+                ngx_log_error(NGX_LOG_ALERT, log, ngx_errno,
+                              ngx_directio_off_n " \"%s\" failed", path.data);
+            }
+
+            directio = 1;
+        }
+
         mp4 = ngx_pcalloc(r->pool, sizeof(ngx_http_mp4_file_t));
         if (mp4 == NULL) {
             return NGX_HTTP_INTERNAL_SERVER_ERROR;
@@ -622,6 +638,7 @@ ngx_http_mp4_handler(ngx_http_request_t 
         mp4->file.fd = of.fd;
         mp4->file.name = path;
         mp4->file.log = r->connection->log;
+        mp4->file.directio = of.is_directio;
         mp4->end = of.size;
         mp4->start = (ngx_uint_t) start;
         mp4->length = length;
@@ -656,23 +673,14 @@ ngx_http_mp4_handler(ngx_http_request_t 
 
     log->action = "sending mp4 to client";
 
-    if (clcf->directio <= of.size) {
-
-        /*
-         * DIRECTIO is set on transfer only
-         * to allow kernel to cache "moov" atom
-         */
+    if (directio) {
+
+        /* DIRECTIO was switched off, restore it */
 
         if (ngx_directio_on(of.fd) == NGX_FILE_ERROR) {
             ngx_log_error(NGX_LOG_ALERT, log, ngx_errno,
                           ngx_directio_on_n " \"%s\" failed", path.data);
         }
-
-        of.is_directio = 1;
-
-        if (mp4) {
-            mp4->file.directio = 1;
-        }
     }
 
     r->headers_out.status = NGX_HTTP_OK;