# HG changeset patch
# User Maxim Dounin <mdou...@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;
