[PATCH 1 of 2] Mp4: fixed directio usage with open_file_cache

[Maxim Dounin]

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1747276031 -10800
#      Thu May 15 05:27:11 2025 +0300
# Node ID 6f0485388f31ead0177fd294f4883b59dfb1dea7
# Parent  4c872940b19b8181d791eb80c17110364150af9d
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.

Note well that even with this fix using directio with open_file_cache
might be problematic on Linux if combined with file AIO or threaded IO,
as directio might be re-enabled after an unaligned read when a thread
tries to do an unaligned read for another request.

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;